WP_User::has_cap and 'map_meta_cap' filter

[dugi digitaly]

add_filter('map_meta_cap', function(){return array();}, 1,0 ); //<-backdoor virus or any plugin
var_dump( user_can( $admin_user_id = 1, 'unavailable cap' ) ); //return true
var_dump( user_can( $Subscriber_user_id = 3, 'remove_users' ) ); //return true

The alternative I propose:
insert if(!in_array($cap,$caps)) return false; inside WP_User::has_cap( $cap ) after 'map_meta_cap' filter
OR
insert if(empty((array)$caps)) return false; inside WP_User::has_cap( $cap ) before the foreach

https://core.trac.wordpress.org/browser/tags/4.1.1/src/wp-includes/capabilities.php#L965

​https://www.diffchecker.com/9cjznf39

[swissspidy]

The attached patch would solve this. All unit tests related to capabilities still pass.

There must be a reason WP_User::has_cap returns true by default though...

[johnbillion]

This'll need security review and tests adding to Tests_User_Capabilities.

[johnbillion]

Replying to swissspidy:

There must be a reason WP_User::has_cap returns true by default though...

Turns out there is. 31518.diff​ breaks Tests_User_Capabilities::test_multisite_user_can_edit_self(). The reason for this is ​this logic in map_meta_cap() which does not add a primitive capability to the $caps array, resulting in it being empty, which results in WP_User::has_cap() now returning false instead of true.

exist is probably the most appropriate capability to require here.

It looks like there are two further conditions inside map_meta_cap() which can result in the $caps array being empty (when a break statement occurs when $caps hasn't been populated):

• (delete|edit)_(post|page} on your own trashed post which was never published (​ref/​ref). Fixed in [35747].
• (edit|delete|add)_post_meta when an auth_post_meta_{$meta_key} filter is in place and returns true (​ref). Invalid.

These will all need fixes and/or unit test coverage before this can go in.

[dugi digitaly]

I'm glad my discovery gained attention.
Good work guys.

[johnbillion]

In 38860:

Role/Capability: Improve the test which asserts that a user can edit their own profile.

All users can edit their own profile, as this ability is not linked to an explicit capability. Technically, it should map to the exist capability, which will be addressed at a later date.

See #31518

[johnbillion]

I believe this change has too high a chance to break something that is relying on the user cap check returning true when the cap check is an empty array. If some malicious code can affect the return value of the map_meta_cap filter, then it can effectively allow anything on the site anyway.

Thanks for the report @dugi digitaly. It lead to a few improvements in the core code and in the unit tests.