Make WordPress Core

Opened 10 years ago

Last modified 5 years ago

#31236 new defect (bug)

wp_ajax_upload_attachment does not properly handle situation when post_id is set

Reported by: johncacpro's profile johncacpro Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.1
Component: Media Keywords: reporter-feedback
Focuses: Cc:

Description

In the wp_ajax_upload_attachment function of wp_admin/includes/ajax_actions.php, it appears that the wrong capability is checked by the current_user_can function if post_id is set in the $_REQUEST object. At line 1845, this code exists:

if ( ! current_user_can( 'edit_post', $post_id ) )

As far as I can tell, edit_post is not a valid capability. I was building a custom post upload for my site that allowed certain users to add media. I had given them the edit_posts capability and they were still receiving the error message "You don't have permission to attach files to this post." Once I changed this line of code in wp-admin to:

if ( ! current_user_can( 'edit_posts', $post_id ) )

it worked as expected.

Attachments (1)

31236.patch (625 bytes) - added by joemcgill 10 years ago.
Fixes incorrect capability check.

Download all attachments as: .zip

Change History (12)

@joemcgill
10 years ago

Fixes incorrect capability check.

#1 @joemcgill
10 years ago

  • Keywords has-patch needs-testing added

#2 @wonderboymusic
10 years ago

  • Milestone changed from Awaiting Review to 4.2

#3 follow-up: @SergeyBiryukov
10 years ago

current_user_can( 'edit_post', $post_id ) is correct, it's used in a lot places in core. It breaks down to edit_posts, edit_published_posts, or edit_others_posts for the post type, see map_meta_cap().

#4 in reply to: ↑ 3 ; follow-up: @joemcgill
10 years ago

Sergey,

I totally trust your judgement on this one, but I'm a bit confused as to how all this is supposed to work. It looks like current_user_can() only accepts one parameter. The WP code reference seems to confirm this but
the codex states otherwise.

Is there something I'm missing or are the docs (and some of the code in core) out of sync with the actual internals for how current_user_can() works?

Replying to SergeyBiryukov:

current_user_can( 'edit_post', $post_id ) is correct, it's used in a lot places in core. It breaks down to edit_posts, edit_published_posts, or edit_others_posts for the post type, see map_meta_cap().

#5 in reply to: ↑ 4 ; follow-up: @helen
10 years ago

Replying to joemcgill:

The magic is in func_get_args(), which allows for variable length arg lists. In whatever decade we'll get to be 5.6+, it'll be easier to understand that from the function definition instead of having to look inside the function. See http://php.net/manual/en/functions.arguments.php#functions.variable-arg-list

Meta caps are quite powerful, as they can be controlled at a very granular level - note the singular name for the capability instead. I find it quite readable - can the current user edit the post with the ID of X. Just don't judge the readability of code that filters meta caps. :)

#6 in reply to: ↑ 5 @joemcgill
10 years ago

Thanks Helen, I learn something new every day. You think it would be worth adding a note in the docs for this?

#7 @helen
10 years ago

Sure, although I'm still curious what the bug is here :)

#8 @joemcgill
10 years ago

  • Keywords reporter-feedback added; has-patch needs-testing removed

Since the bug was reported as "edit_post is not a valid capability", which turns out is untrue, I also am not sure what the bug is here. @johncacpro could you give more insight?

#9 @samuelsidler
9 years ago

  • Milestone changed from 4.2 to Future Release

If we hear back on what the bug here is and if it's an issue introduced in 4.1, we should get this fixed in 4.2. For now, however, punting.

#10 @johncacpro
9 years ago

Here is the scenario that I have currently and what led me to enter the bug. We have a client who wanted a password protected area that allowed registered users the ability to edit their profile information from the website as opposed to logging into Wordpress and doing it there. There are some custom meta fields that are associated with the user accounts and one of them is something like a profile for the user. They wanted the user to be able to have a WYSIWYG editor that allowed them to update this field. Currently, we have embedded the Wordpress editor on a password protected area of the site that allows them to update this field as if they were inside the Wordpress admin area. All of that works fine. The client came back at the end of the project and wanted the users to also be able to upload images to embed in the post. This is where I ran into the issue I reported above. Each user that can do this is in a custom defined role. That custom defined role has the edit_posts capability so in theory they should be able to upload the image to the post, etc. However, whenever I tried to do so, I received an error about not being able to attach files to the post. This led me to track down where this message was being triggered from which I believe is from ajax_actions.php. Once I made the change to the ajax_actions.php file to be edit_posts, the functionality worked as expected.

This was occurring prior to 4.1 as well.

Last edited 9 years ago by johncacpro (previous) (diff)

#11 @michaelbliem
5 years ago

it is a bug, but not because of 'edit_post'-cap.

i run into the same problem and debugged the ajax-call a bit.
problem is, that $_REQUEST('post_id') is set, but with an id that has nothing to do with the call! i have no idea, where this id comes from, seems like random for me!?

because post_id is set, caps are checked of user for this id. when a user doesn't have 'edit_other_posts'-cap, but has 'edit_posts'-cap this call will be declined if he is not post_author of the post with post_id. as post_id is a 'random' id, most of the time this call fails...

Note: See TracTickets for help on using tickets.