Ticket #7277 (closed defect: fixed)

Opened 5 months ago

Last modified 2 months ago

page_options doesn't work for plugin pages

Reported by: Mr Pete Assigned to: ryan
Priority: normal Milestone: 2.7
Component: General Version:
Severity: normal Keywords:
Cc:

Description

According to Codex, one sets up page_options with options to be updated.

This does not pass the check in options.php line 48:

	if( !isset( $whitelist_options[ $option_page ] ) )
		wp_die( __( 'Error! Options page not found.' ) );

	if( $option_page == 'options' ) {
		if( is_site_admin() ) {
			$options = explode(',', stripslashes( $_POST[ 'page_options' ] ));
		} else {
			die( 'Not admin' );
		}
	} else {
		$options = $whitelist_options[ $option_page ];
	}

As coded, page_options only applies to options.php, and $whitelist_options must contain any page name to be used.

Either the code needs to be repaired, or Codex documentation needs to be updated with four bits: - Remove the section on page_options - Revise the discussion of the nonce argument to say it should be called mypage-verb-options (eg. mypage==name of my plugin). - Talk about a requirement to add a filter that augments $whitelist_options - Talk about an input item needed, called option-page (to match the whitelist_options value and the nonce value without the -options suffix)

i.e.

 echo "<input name='option_page' value='myplugin-update' />";
 if (function_exists('wp_nonce_field')) wp_nonce_field('myplugin-update-options');

and code for the new whitelist_options filter

Attachments

optionwhitelist.diff (10.7 kB) - added by donncha on 07/22/08 14:18:40.
Adds option whitelisting

Change History

07/10/08 15:24:42 changed by xknown

  • status changed from new to closed.
  • resolution set to invalid.
  • milestone deleted.

The code you show belongs to WordPress MU and AFAIK, there's a whitelist_options filter where you can modify it.

Please reopen if I'm mistaken.

07/10/08 17:04:15 changed by Mr Pete

  • status changed from closed to reopened.
  • resolution deleted.

Looks like a reasonably significant design difference between MU and 'regular'!

SO... the hard part is the two designs are mutually incompatible in their requirements for the nonce parameter:

MU requires myplugin-update-options (unique per plugin, with a filter) Regular requires update-options (common across all plugins, with POST variable)

Who resolves incompatibilities?

07/19/08 11:51:14 changed by Mr Pete

mu trac entry opened. The design incompatibility is highlighted: http://trac.mu.wordpress.org/ticket/685

07/20/08 10:50:48 changed by donncha

There is an incompatibility but it exists because of a serious hole in MU security. Alex Concha showed that on an MU site any blog admin could change any blog option just by passing the correct list of options and the generic nonce. The admin could change the list of plugins which would allow them to upload a file and add that file to the plugin list (as happened in the most recent round of attacks on WP blogs).

When I added the whitelist to MU I presumed it would end up in WordPress too but I forgot to add a ticket here to discuss those changes.

It's not really an issue for WordPress as the local admin has access to everything anyway. Is it worth discussing merging the whitelist code into WordPress?

07/20/08 11:06:16 changed by westi

If the whitelist code is useful for mu and having in the core means that plugins written for for WP work without changes on mu then lets get the whitelist merged and the info out there for plugin authors.

07/21/08 12:41:52 changed by Mr Pete

Thanks, donncha (and core team!)

For future reference, here is generic backward-compatible plugin code that should handle all three cases correctly: no nonce, old nonce, whitelist nonce.

function myplug_validOptions() {
  global $whitelist_options; 
  $whitelist_options['myplug-update'] = array( 'myplug_opt1','myplug_opt2');
}

global $whitelist_options; // (Need this if inside a function)

if (isset($whitelist_options)) {
  echo '<input name="option_page"  value="myplug-update"           type="hidden" />';
  add_filter('whitelist_options', 'myplug_validOptions'); 
  wp_nonce_field('myplug-update-options');
} else {
  echo '<input name="option_page"  value="update"                  type="hidden" />';
  echo '<input name="page_options" value="myplug_opt1,myplug_opt2" type="hidden" />';
  if (function_exists('wp_nonce_field')) wp_nonce_field('update-options');
}

The code is formatted to highlight the three important bits:

  1. Set option_page ('update' in old scheme, unique in new).
  2. Define valid options via filter (new) or form field (old).
  3. Call wp_nonce_field using option_page value with '-options' appended.

07/22/08 14:18:12 changed by donncha

About to attach "optionwhitelist.diff" which is a diff of the options code in MU trunk and wp.org trunk at rev 8396. This is quite a huge change although I'm not sure how many plugins use options.php to store their options. I never thought of it, so let's hope most other plugin developers didn't either!

07/22/08 14:18:40 changed by donncha

  • attachment optionwhitelist.diff added.

Adds option whitelisting

07/29/08 12:34:41 changed by Mr Pete

Sigh. The code I posted on 07/21/08 needed testing in more environments.

Here's a better sample. NOTE: it's not clear to me what the best test is for the new security system. I'm checking for presence of a function that appears related. In any case, you can't normally test for presence of $whitelist_options in a plugin!

add_filter('whitelist_options', 'myplug_validOptions'); // Add filter globally
function myplug_validOptions() {
  global $whitelist_options; 
  $whitelist_options['myplug-update'] = array( 'myplug_opt1','myplug_opt2');
}

if (function_exists('add_option_whitelist')) {
  echo '<input name="option_page"  value="myplug-update"           type="hidden" />';
  wp_nonce_field('myplug-update-options');
} else {
  echo '<input name="option_page"  value="update"                  type="hidden" />';
  echo '<input name="page_options" value="myplug_opt1,myplug_opt2" type="hidden" />';
  if (function_exists('wp_nonce_field')) wp_nonce_field('update-options');
}

09/04/08 01:10:46 changed by ryan

  • milestone set to 2.7.

09/04/08 01:11:18 changed by ryan

(In [8802]) Add settings registration and whitelisting. Props donncha. see #7277

09/04/08 01:11:42 changed by ryan

  • owner changed from anonymous to ryan.
  • status changed from reopened to new.

Need to add phpdoc and do some cleanup.

09/05/08 18:37:41 changed by westi

(In [8818]) Allow people to update home again! See #7277.

09/29/08 20:49:45 changed by ryan

  • status changed from new to closed.
  • resolution set to fixed.