Make WordPress Core

Opened 3 years ago

Closed 17 months ago

Last modified 17 months ago

#53962 closed defect (bug) (fixed)

The bug allows to see the name(s) of a user(s) who has replied to a comment (not yet authorized).

Reported by: fasuto's profile fasuto Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.2 Priority: normal
Severity: normal Version: 2.7
Component: Comments Keywords: has-patch has-unit-tests has-testing-info commit
Focuses: administration, privacy Cc:

Description

1.- Have a fresh installation of WordPress in its latest version, which comes with a default entry.

2.- Go to the entry and make a comment

3.- The bug, in the navigation bar the following url is placed: http://bug.test/2021/08/19/hola-mundo/?replytocom=2#respond obtaining the response with the username

4.- The comment has not been approved and you can display the user who made it, you can use a script that starts at one and is incremental and you can get the list of users who have made a response to the entry and have not been approved.

Tests performed:

  1. Tested on a WordPress site with Cloudflare protection.
  2. Tests have been performed on WordPress with SSL certificates.

Attachments (1)

Bug Wordpress.pdf (187.0 KB) - added by fasuto 3 years ago.

Download all attachments as: .zip

Change History (40)

@fasuto
3 years ago

#1 follow-up: @peterwilsoncc
3 years ago

  • Component changed from General to Comments
  • Version changed from 5.8 to 2.7

Hello @fasuto and welcome to trac.

Thank you for your report, I am able to reproduce the bug.

It appears to have been introduced in version 2.7 of WordPress, so I've updated the version field of your report to indicate when the bug first appeared.


Notes:

comment_form_title() passes the value of the replytocom querystring parameter to get_comment(). comment_form_title() then uses the parent comment author's name in the title without verifying whether or not the comment has been approved.

The same is true for get_comment_id_fields().

#2 in reply to: ↑ 1 ; follow-up: @fasuto
3 years ago

Hello @peterwilsoncc

Hi thanks for replying, the bug could allow a security breach by listing the users commenting on the post, I wanted to report it by hacker one but couldn't, I hope it can be fixed.

Replying to peterwilsoncc:

Hello @fasuto and welcome to trac.

Thank you for your report, I am able to reproduce the bug.

It appears to have been introduced in version 2.7 of WordPress, so I've updated the version field of your report to indicate when the bug first appeared.


Notes:

comment_form_title() passes the value of the replytocom querystring parameter to get_comment(). comment_form_title() then uses the parent comment author's name in the title without verifying whether or not the comment has been approved.

The same is true for get_comment_id_fields().

#3 in reply to: ↑ 2 @peterwilsoncc
3 years ago

  • Milestone changed from Awaiting Review to 5.9

Replying to fasuto:

... the bug could allow a security breach by listing the users commenting on the post, I wanted to report it by hacker one but couldn't, I hope it can be fixed.

I thought about that and decided that it can be worked on in public in a similar way that #49956 was.

As comments (including the commenter's name) are intended to be public, I don't think there is much concern about exposing data intended to be private. The main issue here is making sure it's not exploited by spammers.

I've just moved it on to the next major release's milestone for visibility.

#4 @fasuto
3 years ago

thank you @peterwilsoncc, see you next time!

This ticket was mentioned in PR #1607 on WordPress/wordpress-develop by costdev.


3 years ago
#5

  • Keywords has-patch added

comment_form_title() and get_comment_id_fields() allows replies to unapproved comments via ?replytocom=[comment-id]#respond.

This patch checks that the comment has been approved before outputting the unapproved comment's $author and the comment_parent hidden field.

Trac ticket: https://core.trac.wordpress.org/ticket/53962

#6 @costdev
3 years ago

  • Keywords has-unit-tests added

peterwilsoncc commented on PR #1607:


3 years ago
#7

Thanks for updating @costdev, it will probably be a Monday next week (Aus time) before I can pull down the branch for testing. I've set a reminder to do so then.

costdev commented on PR #1607:


3 years ago
#8

Sounds good @peterwilsoncc!

This ticket was mentioned in Slack in #core-test by peterwilsoncc. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#11 @hellofromTonya
3 years ago

  • Keywords has-testing-info added

Reproduce/testing instructions are included in the ticket's description.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#15 @Boniu91
3 years ago

Test Report

Env

  • WordPress PR1607
  • Chrome 92.0.4515.159
  • Windows 10
  • Theme: Twenty Twenty One
  • Gutenberg Editor
  • Plugin: WP Downgrade

Steps tested before applying fix

  1. Added new comment, which is not approved
  2. Visited /2021/04/16/hello-world/?replytocom=2#respond
  3. I was redirected to adding a reply to specific person (his nickname is not visible in the comment list, so is the comment), nickname of unapproved author was displayed in the reply form.

Steps tested after applying fix

  1. Added new comment, which is not approved
  2. Visited /2021/04/16/hello-world/?replytocom=2#respond
  3. I was correctly redirected to adding a new reply, nickname of unapproved author wasn't displayed
  4. Approved the comment and moved it to spam and trash and checking the behaviour in the meantime

The fix looks good to me.

costdev commented on PR #1607:


3 years ago
#16

@hellofromtonya I think that's all of the changes made if you're available for another review :-)

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 years ago

#18 @hellofromTonya
3 years ago

  • Milestone changed from 5.9 to 6.0
  • Owner set to hellofromTonya
  • Status changed from new to accepted

@costdev and @fasuto, I'm sorry for dropping the ball on reviewing and testing PR 1607. The PR is close but there are open discussions happening in it and it needs another round of thorough testing and code review. With 5.9 Beta 1 tomorrow, likely time has run out for this to ship in 5.9. Again I'm sorry for dropping the ball.

I'll move it to 6.0 and assign myself as owner to shepherd getting it completed and committed. If it does get done before Beta 1 starts, happy to pull it back into 5.9.

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#20 @costdev
2 years ago

  • Milestone changed from 6.0 to 6.1

Per the discussion in the bug scrub, I'm moving this ticket to the 6.1 milestone as it has not had any movement and needs further testing and code review.

If this ticket makes progress towards resolution during the 6.0 cycle, feel free to bring it back into the milestone for commit consideration.

#21 @milana_cap
22 months ago

  • Keywords add-to-field-guide added

#22 @hellofromTonya
22 months ago

  • Keywords changes-requested added

Tomorrow is 6.1 RC1. There's been no movement on this ticket for over a year. What's needed?

The PR:

  • has changes requested
  • will need to be rebased to the current trunk
  • will need a deep review
  • will need another round of testing once it's ready

Moving it to 6.2 in the hopes that it can move forward in the next cycle.

#23 @hellofromTonya
22 months ago

  • Milestone changed from 6.1 to 6.2

This ticket was mentioned in Slack in #core by costdev. View the logs.


18 months ago

@peterwilsoncc commented on PR #1607:


17 months ago
#25

Looks like my curl request got stripped/I had a silly moment.

This is what I did, comment 6 was unapproved

curl 'http://wp-dev.local/wp/wp-comments-post.php' -X POST -H 'Referer: http://wp-dev.local/2023/02/01/hello-world/' -H 'Content-Type: application/x-www-form-urlencoded' --data-raw 'comment=An+unapproved+comment&author=Peter&email=wilson%40example.com&url=&submit=Post+Comment&comment_post_ID=1&comment_parent=6'

#26 follow-up: @peterwilsoncc
17 months ago

  • Keywords commit added

I've approved the linked pull request as of fe1f2cf8a57.

@hellofromTonya Are you able to take a look at the PR as ticket owner?

#27 @hellofromTonya
17 months ago

  • Keywords changes-requested removed

Reviewing for commit.

#28 in reply to: ↑ 26 @hellofromTonya
17 months ago

Replying to peterwilsoncc:

I've approved the linked pull request as of fe1f2cf8a57.

@hellofromTonya Are you able to take a look at the PR as ticket owner?

Thanks @peterwilsoncc! Yes, I will.

@peterwilsoncc commented on PR #1607:


17 months ago
#29

@costdev I've applied the suggestions from Tonya's review and pushed to this branch.

I haven't rebased against trunk as that would ruin your local branch and I didn't want to do that.

#30 @peterwilsoncc
17 months ago

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

In 55369:

Comments: Prevent replying to unapproved comments.

Introduces client and server side validation to ensure the replytocom query string parameter can not be exploited to reply to an unapproved comment or display the name of an unapproved commenter.

This only affects commenting via the front end of the site. Comment replies via the dashboard continue their current behaviour of logging the reply and approving the parent comment.

Introduces the $post parameter, defaulting to the current global post, to get_cancel_comment_reply_link() and comment_form_title().

Introduces _get_comment_reply_id() for determining the comment reply ID based on the replytocom query string parameter.

Renames the parameter $post_id to $post in get_comment_id_fields() and comment_id_fields() to accept either a post ID or WP_Post object.

Adds a new WP_Error return state to wp_handle_comment_submission() to prevent replies to unapproved comments. The error code is comment_reply_to_unapproved_comment with the message Sorry, replies to unapproved comments are not allowed..

Props costdev, jrf, hellofromtonya, fasuto, boniu91, milana_cap.
Fixes #53962.

#32 @hellofromTonya
17 months ago

Thanks for committing this @peterwilsoncc.

Double checking on a question I asked in the PR:

Is it intentional that setting the global $comment is now removed from comment_form_title()?

Are there any BC concerns for removing it?

cc @costdev

This ticket was mentioned in PR #4110 on WordPress/wordpress-develop by @costdev.


17 months ago
#33

In [55369], the global $comment assignment was mistakenly removed.

This restores the assignment.

Follow-up to [55369].

Trac ticket: https://core.trac.wordpress.org/ticket/53962

#34 @costdev
17 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

As noted by @hellofromTonya, [55369] removed the global $comment assignment in comment_form_title(). The @internal annotation of comment_form_title() states:

@internal The $comment global must be present to allow template tags access to the current comment. See https://core.trac.wordpress.org/changeset/36512.

PR 4110 restores the global $comment assignment.

Note: As this leads to two calls to get_comment() - one in comment_form_title(), and one in _get_comment_reply_id(), a follow-up investigation is needed to see if we can reduce this to one call.

For example, this may require changing _get_comment_reply_id() to _get_comment_reply_object(), and changing all uses to perform appropriate checks before an effective _get_comment_reply_object()->ID call.

#35 @hellofromTonya
17 months ago

Yes, PR 4110 restores the global $comment assignment in comment_form_title() for backwards-compatibility.

Prepping commit.

This ticket was mentioned in Slack in #core by costdev. View the logs.


17 months ago

#37 @hellofromTonya
17 months ago

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

In 55395:

Comments: Restore global $comment assignment in comment_form_title().

Restores the global $comment assignment in comment_form_title(), which was mistakely removed in [55369].

As noted in the function's DocBlock:

@internal The $comment global must be present to allow template tags access to the current comment. See https://core.trac.wordpress.org/changeset/36512.

Follow-up to [55369].

Props hellofromTonya, costdev.
Fixes #53962.

#39 @milana_cap
17 months ago

  • Keywords add-to-field-guide removed
Note: See TracTickets for help on using tickets.