23

As suggested in this question, I am adding this topic as a new question, for community discussion/voting regarding best-practices for Plugin/Theme security.

Here is the starting checklist, based on my current (work-in-progress) settings/data security checklist used for reviewing Themes (the principles should be no different for Plugins than they are for Themes).

If you want to check out a theme with a secure and solidly-coded theme settings page, check out this theme:
http://wordpress.org/extend/themes/coraline

Improve this question
4
  • If someone with the appropriate privileges wouldn't mind making this a community wiki?
    – Chip Bennett
    Mar 30, 2011 at 21:04
  • To get a question in wiki mode a mod needs to tag the question appropriately, i've flagged it for mod attention, it's a just a matter of time now.. :)
    – t31os
    Mar 30, 2011 at 21:29
  • What's so special about coraline? Imo there are still ways to get in. I'd suggest to link anons A insted: wordpress.stackexchange.com/questions/13539/…
    – kaiser
    Apr 1, 2011 at 3:34
  • There may be nothing particularly special about Coraline. It's simply the one we currently point Theme developers to, when reviewing Themes, because it was the example given by Justin Tadlock, who did many of the initial security-specific Theme reviews. I'd offer up Oenology also as a good example, but I don't want to come across as trying to pimp my own Theme. :)
    – Chip Bennett
    Apr 1, 2011 at 12:59

13 Answers 13

Reset to default
12

Sanitize, validate and escape data

Sanitize anything that could get in and out of (!) the database both front end and back end!

Plugins and Themes should perform proper data validation:

  1. Validate and sanitize all untrusted data before entering data into the database
  2. Escape all untrusted data before being output in the Settings form fields

  3. Escape all untrusted data before being output in the Theme template files

Plugins and Themes should use esc_attr() for text inputs and esc_html() or esc_textarea() for text areas.

Also available from the WordPress API is esc_url(), esc_url_raw(), esc_js() and wp_filter_kses().

Bad example:

<?php $url = 'javascript:pwnd()'; ?>
<a href="<?php echo $url; ?>">anchor</a>

Good example:

<a href="<?php echo esc_url($url); ?>">anchor</a>

Here is a great video of Mark Jaquith explaining the usage of the escaping functions:

Improve this answer
1
  • 3
    Sanitize on the way in, escape on the way out.. :)
    – t31os
    Mar 30, 2011 at 22:04
9

Only use $_GET / $_POST / $_REQUEST carefully and when better APIs are unavailable

Plugins and Themes should use the Settings API to get and save form input data rather than rely on $_POST and $_REQUEST data directly.

Improve this answer
2
  • 3
    Always treat $_POST, $_REQUEST, and $_GET as unsafe. Sanitize and whitelist values from those arrays and put them into your own variables. Never sanitize values coming from the user and put them back into $_POST.
    – goldenapples
    Mar 30, 2011 at 22:49
  • 2
    Always check that the key that you need to use is set in the appropriate array. isset() is your friend :)
    – mfields
    Apr 18, 2011 at 8:32
9

Use $wpdb->prepare

When building custom queries via the $wpdb object, always use $wpdb->prepare to fill placeholders with values instead of writing the queries with data mixed with SQL code, as the mysql_* family functions have wrongly taught everyone.

Improve this answer
1
  • $wpdb->prepare is not the same as prepared statements.
    – hakre
    Aug 17, 2011 at 12:01
8

Be careful with PHP functions that might be used to run malicious code

A nice read for anybody writing PHP: Exploitable PHP functions on StackOverflow.

Use Theme Modification API

Themes should use set_theme_mod() and related functions not an self invented name scheme.
The theme_mod API is a specialized layer for the settings API; it guarantees unique names, pushes all options into one array and is – from my experience – much easier to handle. Plus, it offers standardized filters for plugins – which is good for interoperability.

Avoid enabled register_globals

Don’t rely on register_globals = on. A Pro Theme which my last client bought does exactly this. I could hack any site using this theme in 5 minutes …
ThimbThumb did this too (and still does?).

Do not create files with unnecessary wide access permissions

Don’t create files with too liberate access permissions.

Use SSL where available

Point your Share on Twitter/Facebook/Anything links to the HTTPS URI if available. Your reader’s security is important too.

Improve this answer
4
  • 2
    Can you elaborate on set_theme_mod(), especially, how properly to combine it with Settings API usage?
    – Chip Bennett
    Mar 30, 2011 at 22:27
  • @Chip Bennett I have added some information to my answer.
    – fuxia ♦
    Mar 30, 2011 at 22:42
  • Can you break down this mini-list in smaller more specific answers? Easier to manage community wiki in smaller chunks. TIA
    – Rarst
    Mar 31, 2011 at 15:23
  • 3
    Chip: The Theme mod system doesn't integrate too well with the Settings API. I'll write up a post explaining how to do it right sometime soon.
    – Otto
    Apr 1, 2011 at 22:17
7

Save data in single array

Plugins and Themes should save options in a single array, rather than create multiple options for the settings page. The use of the Settings API would handle this.

Improve this answer
6

Check for appropriate capability when adding and outputting settings pages

Plugins should use an appropriate capability (e.g. manage_options) for the capability to add the settings page.

Themes should use edit_theme_options as the appropriate capability to add the settings page.

Improve this answer
3
  • 1
    Small but important note: You can't use edit_theme_options with the settings API though, option submission is hard-coded to require manage_options in order to submit updates. Related Trac ticket can be found here.
    – t31os
    Mar 30, 2011 at 21:35
  • True, but 1) that will only impact Editors, not Admins; and 2) hopefully that will be addressed soon, via the linked Trac ticket.
    – Chip Bennett
    Mar 30, 2011 at 21:56
  • There's always the possibility of a custom role or a regular role being given the edit_theme_options cap to, i thought it might be handy to point out that the settings API in it's current state is only usable by roles with the manage_options capability.
    – t31os
    Mar 30, 2011 at 22:01
5

Use up-to-date tutorials and information

Plugins & Themes should both implement Options and Settings pages deliberately and not rely on copy-and-paste website tutorials which are outdated and do not include proper data security, such as those listed below.

Examples of what not to do:

Improve this answer
2
  • 1
    I added a little emphasized text in to indicate the links are examples of what not to do, as it's easy to skim through the information and click the links without reading the paragraph that preceeds them. Also made the answer a little prettier whilst i was there... ;)
    – t31os
    Mar 30, 2011 at 21:27
  • 2
    This could use bit of explanation what exactly example tutorials are doing in a wrong and/or old way.
    – Rarst
    Mar 31, 2011 at 20:24
4

Use Settings API

Plugins and Themes should use the Settings API, which is easier to use, more secure, and takes care of a lot of the hard work of settings pages:

For a good tutorials on using the Settings API, see:

Improve this answer
1
  • See my comment on this answer with regards to the settings API and theme options.
    – t31os
    Mar 30, 2011 at 21:36
1

For checkboxes and select options, Plugins and Themes should use the checked() and selected() functions for outputting checked="checked" and selected="selected", respectively.

Improve this answer
4
  • Not really a security thing, unless I am missing something. Still highly convenient and good to use. :)
    – Rarst
    Mar 31, 2011 at 20:39
  • Well, perhaps and perhaps not. I've seen a LOT of custom code for accomplishing the same thing. More code spaghetti = more opportunities to introduce security risks. :)
    – Chip Bennett
    Mar 31, 2011 at 20:49
  • Bennet - from mailing with toscho some days ago - i guess i can say it for both of us - most simple functions for that are much easier to read & understand than those functions. I'm not down-, but also not upvotings this. Imo this shouldn't be part of the core, as it doesn't add any value.
    – kaiser
    Apr 1, 2011 at 3:29
  • 2
    I'm curious what you guys came up with that's easier than checked( $theme_options['whatever_option'] ) or checked( 'some_value' == $theme_options['whatever_option'] ). I don't know how it gets more succinct than that?
    – Chip Bennett
    Apr 1, 2011 at 12:55
1

Prefix function and variable names

Plugins should prefix all options, custom functions, custom variables, and custom constants with plugin-slug.

Themes should prefix all options, custom functions, custom variables, and custom constants with theme-slug.

Improve this answer
1
  • I would extend this to all class names as well as names of custom objects like post_types and taxonomies.
    – mfields
    Apr 18, 2011 at 8:29
1

Use wp_safe_redirect() instead of calling php's header() function directly when directing to a page on the same domain.

Improve this answer
0
0

Add settings pages to appropriate sections of admin menu

Plugins should use the add_options_page() function to add the Plugin Settings Page to the Settings menu, rather than using add_menu_page() to add a top-level menu.

Themes should use the add_theme_page() function to add the Theme Settings Page to the Appearance menu, rather than using add_menu_page() to add a top-level menu.

Improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.