Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#16129 closed defect (bug) (fixed)

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

Reported by: 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 5 years ago.
16129.patch (541 bytes) - added by casben79 5 years ago.
Patch File
16129 - 2.patch (794 bytes) - added by casben79 5 years ago.
Second Attempt
16129 - 3.patch (789 bytes) - added by casben79 5 years ago.
Whitespace Added , String Corrected
16129 - 3.2.patch (791 bytes) - added by casben79 5 years ago.
Updated Whitespace
16129 - 3.2.2.patch (721 bytes) - added by casben79 5 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 @ocean905 years ago

  • Description modified (diff)

@casben795 years ago


@casben795 years ago

Patch File

comment:2 @casben795 years ago

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

comment:3 @casben795 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 :)


comment:4 @garyc405 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.

comment:5 follow-up: @casben795 years ago


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

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

Safe to remove it?

comment:6 in reply to: ↑ 5 @SergeyBiryukov5 years ago

Replying to casben79:

Safe to remove it?

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

comment:7 @casben795 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.

comment:8 @SergeyBiryukov5 years ago

Yep, safe to remove then, I guess.

@casben795 years ago

Second Attempt

comment:9 @casben795 years ago

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

comment:10 @nacin5 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.

@casben795 years ago

Whitespace Added , String Corrected

comment:11 @casben795 years ago

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

Version 0, edited 5 years ago by casben79 (next)

comment:13 @casben795 years ago

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

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

comment:14 @nacin5 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.

@casben795 years ago

Updated Whitespace

@casben795 years ago

comment:15 @casben795 years ago

I think Thats It :)

comment:16 @nacin5 years ago

  • Keywords commit added

comment:17 @ryan5 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.