WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#17668 closed defect (bug) (fixed)

Revisions should require same caps as parents for read/edit/delete

Reported by: ejdanderson Owned by: markjaquith
Milestone: 3.2 Priority: normal
Severity: minor Version: 3.2
Component: Revisions Keywords: 2nd-opinion has-patch
Focuses: Cc:

Description (last modified by aaroncampbell)

wp_post_revision_title is displaying the post edit link based off of a user's edit_post capability for the revision post type, not it's parent's post type.

The issue resides in the get_edit_post_link method, where it checks on the given post type's capability.

I've attached a simple fix.

EDIT: It seems that revisions always use (read|edit|delete)_post for cap checks even if the post-type of their parent uses something custom. This results in users that are able to read/edit/delete revisions of posts that they don't have caps to read/edit/delete

Attachments (3)

link-template.diff (673 bytes) - added by ejdanderson 3 years ago.
17668.diff (1.1 KB) - added by aaroncampbell 3 years ago.
cpt-cap-test.php (1.4 KB) - added by aaroncampbell 3 years ago.

Download all attachments as: .zip

Change History (13)

ejdanderson3 years ago

comment:1 dd323 years ago

  • Keywords has-patch added

comment:2 follow-up: dd323 years ago

  • Milestone changed from Awaiting Review to 3.2

pushing to 3.2 for review. I'm not sure the patch is covering it 100%, It looks like the post type object needs to be changed to the parent type completely rather than the revision type to me.

comment:3 in reply to: ↑ 2 aaroncampbell3 years ago

Replying to dd32:

It looks like the post type object needs to be changed to the parent type completely rather than the revision type to me.

Since a custom post type can specify an edit_post cap, it could definitely be different from the the edit_post cap of revisions. We'll need to get the parent_post_type->cap->edit_post and pass that to current_user_can.

I'm going to look into a solution now.

comment:4 follow-up: aaroncampbell3 years ago

  • Keywords 2nd-opinion added; has-patch removed

The issue is deeper than just get_edit_post_link. The truth is that you can edit a revision even if you don't have the rights to edit it's parent. If we're wanting to change this it's not going to be a simple fix like what's proposed here. It even looks like there are some places that are using 'edit_post' instead of post_type->cap->edit_post. It looks like the code would look like this:

if ( 'revision' == $post->post_type ) {
	$post_cap_id = $post->post_parent;
	$parent_post = get_post( $post_cap_id );
	$post_type_object = get_post_type_object( $parent_post->post_type );
} else {
	$post_cap_id = $post->ID;
	$post_type_object = get_post_type_object( $post->post_type );
}
if ( !$post_type_object )
	return;

if ( !current_user_can( $post_type_object->cap->edit_post, $post_cap_id ) )
	return;

But it looks like we would need this in quite a few places. Grepping around for a few minutes turned up these:

  • get_edit_post_link()
  • wp_post_revision_title() - Here we could check the return of get_edit_post_link
  • get_inline_data()
  • post_preview() - looks like it might use 'edit_post' incorrectly
  • edit_post()
  • _wp_translate_postdata() - looks like it might use 'edit_post' incorrectly
  • WP_Posts_List_Table::single_row()
  • WP_Posts_List_Table::ajax_user_can() - This seems to JUST check the current $post_type_object so we don't really have a parent to check if it's revision
  • wp-admin/revision.php?action=restore - looks like it might use 'edit_post' incorrectly
  • wp-admin/post.php?action=edit

Obviously some of these might have checks prior to what I was looking at (and it's been a really long day, so I'm going a bit cross-eyed), but the rabbit hole seems to be deeper than it looked.

comment:5 in reply to: ↑ 4 ; follow-up: ejdanderson3 years ago

Replying to aaroncampbell:

The issue is deeper than just get_edit_post_link. The truth is that you can edit a revision even if you don't have the rights to edit it's parent.

Perhaps a solution resides in map_meta_cap() with checking the parent's capabilities on the 'edit_post', 'delete_post', and 'read_post' cases? Shouldn't revisions always inherit all of their parent post type's relevant capabilities?

comment:6 in reply to: ↑ 5 aaroncampbell3 years ago

  • Keywords has-patch added

Replying to ejdanderson:

Perhaps a solution resides in map_meta_cap() with checking the parent's capabilities on the 'edit_post', 'delete_post', and 'read_post' cases?

That's a great idea. Patch coming right up.

Replying to ejdanderson:

Shouldn't revisions always inherit all of their parent post type's relevant capabilities?

I'm not sure. I honestly can't tell if this is an oversight, something we never made it to (but planned to), or expected behavior. I'll ask around.

aaroncampbell3 years ago

comment:7 aaroncampbell3 years ago

  • Description modified (diff)
  • Summary changed from wp_post_revision_title capabilities to Revisions should require same caps as parents for read/edit/delete

comment:8 johnjamesjacoby3 years ago

  • Cc jjj@… added

CC'ing myself for future testing with bbPress 2.0+

aaroncampbell3 years ago

comment:9 nacin3 years ago

I agree with the approach that 17668.diff takes. I've seen this before and even written the same patch, but not sure why I failed to pursue it.

comment:10 markjaquith3 years ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from new to closed

In [18200]:

Check parent caps for revisions. props aaroncampbell. fixes #17668

Note: See TracTickets for help on using tickets.