![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 11 years ago
Closed 8 years ago
#31518 closed defect (bug) (wontfix)
WP_User::has_cap and 'map_meta_cap' filter
| Reported by: | dugi digitaly | Owned by: | johnbillion |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Role/Capability | Version: | 2.0 |
| Severity: | normal | Keywords: | needs-patch has-unit-tests early |
| Cc: | Focuses: |
Description (last modified by )
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
Attachments (2)
Change History (12)
#4
@
11 years ago
- Keywords needs-unit-tests added
- Milestone Awaiting Review → Future Release
- Owner set to
- Status new → reviewing
This'll need security review and tests adding to Tests_User_Capabilities.
#5
in reply to: ↑ 1
@
11 years ago
- Keywords needs-patch added; has-patch dev-feedback removed
Replying to swissspidy:
There must be a reason
WP_User::has_capreturnstrueby 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).(edit|delete|add)_post_metawhen anauth_post_meta_{$meta_key}filter is in place and returns true (ref).
These will all need fixes and/or unit test coverage before this can go in.
#10
@
8 years ago
- Milestone Future Release
- Resolution → wontfix
- Status accepted → closed
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.
The attached patch would solve this. All unit tests related to capabilities still pass.
There must be a reason
WP_User::has_capreturnstrueby default though...