add_menu_page Security Bug

[shazahm1@…]

I think there is a security issue with the add_menu_page() function but I'm a noob and might be doing something wrong but I was able to duplicate it with the sample code from the codex.

<?php
/*
Plugin Name: Menu Test
Plugin URI: http://wordpress.org
Description: Menu Test
Author: Nobody
Author URI: http://example.com
*/

// Hook for adding admin menus
add_action('admin_menu', 'mt_add_pages');

// action function for above hook
function mt_add_pages() {
    // Add a new submenu under Options:
    add_options_page('Test Options', 'Test Options', 8, 'testoptions', 'mt_options_page');

    // Add a new submenu under Manage:
    add_management_page('Test Manage', 'Test Manage', 8, 'testmanage', 'mt_manage_page');

    // Add a new top-level menu (ill-advised):
    add_menu_page('Test Toplevel', 'Test Toplevel', 8, __FILE__, 'mt_toplevel_page');

    // Add a submenu to the custom top-level menu:
    add_submenu_page(__FILE__, 'Test Sublevel', 'Test Sublevel', 8, 'sub-page', 'mt_sublevel_page');

    // Add a second submenu to the custom top-level menu:
    add_submenu_page(__FILE__, 'Test Sublevel 2', 'Test Sublevel 2', 8, 'sub-page2', 'mt_sublevel_page2');
}

// mt_options_page() displays the page content for the Test Options submenu
function mt_options_page() {
    echo "<h2>Test Options</h2>";
}

// mt_manage_page() displays the page content for the Test Manage submenu
function mt_manage_page() {
    echo "<h2>Test Manage</h2>";
}

// mt_toplevel_page() displays the page content for the custom Test Toplevel menu
function mt_toplevel_page() {
    echo "<h2>Test Toplevel</h2>";
}

// mt_sublevel_page() displays the page content for the first submenu
// of the custom Test Toplevel menu
function mt_sublevel_page() {
    echo "<h2>Test Sublevel</h2>";
}

// mt_sublevel_page2() displays the page content for the second submenu
// of the custom Test Toplevel menu
function mt_sublevel_page2() {
    echo "<h2>Test Sublevel 2</h2>";
}

?>

Let's say a user is logged in as a subscriber and types in the query string to access the top level menu ?page=menu_test.php the page will be displayed even though only admins should see the page as set in the parameter set in add_menu_page. However type in the query string for the subpages are blocked as expected. I've also attached my code that shows the same problem.

[ryan]

Try with the 2.8.1 beta, which enforces more strictly. Plugins should always do explicit capability checks, regardless.

[Denis-de-Bernardy]

as admin in 2.8.1-beta-2, I can't access an admin page that isn't registered explicitly.

[hakre]

(for me the same)

Just installed your plugin.

/wordpress-trunk/wp-admin/admin.php?page=menu_test.php gives me a WP_Die with this message:

You do not have sufficient permissions to access this page.

I am admin. When logged out WP display the login form.

So this is worksforme because I'm unable to reproduce.

Tested against current trunk. Maybe you have got other Plugins activated, not the default Theme?

[Denis-de-Bernardy]

he's testing using 2.8, and his bug works then indeed.

[shazahm1@…]

I just installed 2.8.1-beta2 and retested as suggested. I can no longer access the page using the query string as I reported. However this can be reproduced in both released versions of 2.7 and 2.8.

[hakre]

As ryan properly pointed out, it is the plugin page itself that needs to check for propper access rights.

[shazahm1@…]

Understood... I'm off to Google to see how this is done properly. None of the tutorials have come across so far mentioned needing an additional check. I know I should use current_user_can and wp_die. I'm just not exactly sure where the right place to use.

[hakre]

see: ​http://codex.wordpress.org/Adding_Administration_Menus
see: ​http://codex.wordpress.org/Creating_Options_Pages#Register_Settings

[DeNeusbeer]

I recently encountered this bug again in WP 2.9.1. I reproduced it using a fresh install of wordpress and this very simple plugin i wrote. It only adds 1 menu page for Administrators, and 1 submenu page for Contributors:

<?php
/*
Plugin Name: Menu testing
Plugin URI: 
Description: Testing menu page structure
Author: DeNeusbeer
*/
 
add_action('admin_menu', 'admin_menu_custom');
 
 
function admin_menu_custom() {
    
    add_menu_page('Adminpage', 'Adminpage', 'administrator', 'adminpage', 'get_adminpage');
    
    add_submenu_page('index.php', 'Testpage', 'Testpage', 'contributor', 'testpage', 'get_contributor_page');
}
 
function get_adminpage() {
    echo 'This is an administrator page, only administrators should see it.';    
}
    
function get_contributor_page() {
    echo 'this is a contributor page.';
}
?>

I added 2 users, one Administrator and one Contributor. The normal link to the newly created Adminpage menu is /wp-admin/admin.php?page=adminpage and this is accesible only be the Administrator, as expected. But if i point the browser to /wp-admin/index.php?page=adminpage , the Contributor can see the page aswell.

This only occurs for menu pages, not for submenu pages.

[ryan]

'administrator' and 'contributor' are not valid capabilities. They are roles. That could be the problem.

Could you create a new ticket for this? We prefer not to reopen old tickets, and creating a new one will give the issue more visibility.

[miqrogroove]

They are roles. That could be the problem.

Nah, the $access_level parameter has never been implemented for add_menu_page. wp-admin/menu.php displays all top level menus unless all children are forbidden. The hooks for those top level pages are totally unchecked.

[miqrogroove]

Replying to hakre:

/wordpress-trunk/wp-admin/admin.php?page=menu_test.php gives me a WP_Die

Looks like that test was bogus. It doesn't match anything in the plugin.

+1 to reopen this ticket instead of making a new one.

[miqrogroove]

Wow it's happening with some roles and not others. Isn't that something...

[nacin]

Replying to miqrogroove:

Wow it's happening with some roles and not others. Isn't that something...

What you might be noticing is this. On an admin user:

var_dump( current_user_can('administrator') ); // bool(true)
var_dump( current_user_can('editor') ); // bool(false)

Which seems to makes sense, as we don't compare the caps assigned to two roles in core.

[miqrogroove]

Okay I'm starting to see significant differences in the test cases, so for the sake of clarity, here's the separate ticket:

#11922

Enjoy!