Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#16129 closed defect (bug) (fixed)

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

Reported by: westi's profile westi Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description (last modified by ocean90)

Checks current_user_can( 'edit_post', $comment_post_ID )

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

Attachments (6)

16129.diff (4.6 KB) - added by casben79 13 years ago.
Changed
16129.patch (541 bytes) - added by casben79 13 years ago.
Patch File
16129 - 2.patch (794 bytes) - added by casben79 13 years ago.
Second Attempt
16129 - 3.patch (789 bytes) - added by casben79 13 years ago.
Whitespace Added , String Corrected
16129 - 3.2.patch (791 bytes) - added by casben79 13 years ago.
Updated Whitespace
16129 - 3.2.2.patch (721 bytes) - added by casben79 13 years ago.

Download all attachments as: .zip

Change History (23)

#1 @ocean90
13 years ago

  • Description modified (diff)

@casben79
13 years ago

Changed

@casben79
13 years ago

Patch File

#2 @casben79
13 years ago

  • Cc bcasey@… added
  • Keywords has-patch added

#3 @casben79
13 years ago

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

#4 @garyc40
13 years ago

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.

#5 follow-up: @casben79
13 years ago

Oops:

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

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

Safe to remove it?

#6 in reply to: ↑ 5 @SergeyBiryukov
13 years ago

Replying to casben79:

Safe to remove it?

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

#7 @casben79
13 years ago

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.

#8 @SergeyBiryukov
13 years ago

Yep, safe to remove then, I guess.

@casben79
13 years ago

Second Attempt

#9 @casben79
13 years ago

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

#10 @nacin
13 years ago

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
13 years ago

Whitespace Added , String Corrected

#11 @casben79
13 years ago

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

Also do I hit commit or is that not my place?

Last edited 13 years ago by casben79 (previous) (diff)

#13 @casben79
13 years ago

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

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

#14 @nacin
13 years ago

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
13 years ago

Updated Whitespace

#15 @casben79
13 years ago

I think Thats It :)

#16 @nacin
13 years ago

  • Keywords commit added

#17 @ryan
13 years ago

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

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

Note: See TracTickets for help on using tickets.