Incorrect cap check in wp-admin/includes/comment.php : edit_comment()

[westi]

Checks current_user_can( 'edit_post', $comment_post_ID )

But should be current_user_can( 'edit_comment', (int) $_POST['comment_ID'] )

[casben79]

Changed

[casben79]

Patch File

[casben79]

This is my first attempt at helping out with WP, if i did anything wrong, let me know. Thought this would be a nice easy one to learn on :)

Cheers
Ben

[garyc40]

I haven't tested the patch yet, but at a glance, it should be:

if ( !current_user_can( 'edit_comment', (int) $_POST['comment_ID'] ) ) 

Otherwise you're passing the ID of the post containing the comment.

[casben79]

Oops:

with that change, I see no point in the following line:

$comment_post_ID = (int) $_POST['comment_post_ID'];

Safe to remove it?

[SergeyBiryukov]

Replying to casben79:

Safe to remove it?

Note the difference between $_POST['comment_ID'] and $_POST['comment_post_ID'].

[casben79]

Yes, comment_post_ID is the post id that is being commented on.

Seeing as we are no longer using $comment_post_ID I see no point in creating the variable in the first place, it is not used anywhere else in the function Nor returned in any way.

[SergeyBiryukov]

Yep, safe to remove then, I guess.

[casben79]

Second Attempt

[casben79]

Also changed wp_die wording ALA: ​http://core.trac.wordpress.org/ticket/16061 While I was There.

[nacin]

We're into release candidate stage, which means we're no longer creating new strings for translators.

Here is a string that already exists for now: "You are not allowed to edit comments on this post."

That, and this needs whitespace: if (!current_user_can( 'edit_comment', (int)$_POST['comment_ID'] )) should be if ( ! current_user_can( 'edit_comment', (int) $_POST['comment_ID'] ) ).

Patch looks good with those changes.

[casben79]

Whitespace Added , String Corrected

[casben79]

Not Going to argue , Just out of curiosity, why the need for whitespace?

[nacin]

That's our coding standard. ​http://codex.wordpress.org/WordPress_Coding_Standards#Space_Usage

[casben79]

Hmm, Fair enough. Learn about 10 new things every day .

Thank you for persisting, hopefully be a smoother process in the future :)

[nacin]

While we have hundreds of contributors, less than a dozen have commit access. A committer will pull the trigger here after a review. :)

In the meantime, if you'd like to refresh the patch just one more time, the opening quote on the line with wp_die() needs a space before it. Also, you have some trailing whitespace on the blank line that needs to be removed.

[casben79]

Updated Whitespace

[casben79]

I think Thats It :)

[ryan]

(In [17232]) Use edit_comment cap. Props casben79. fixes #16129