WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 3 weeks ago

#53784 assigned defect (bug)

Limiting user enumeration through the REST API

Reported by: ehtis Owned by:
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch has-unit-tests has-screenshots dev-feedback
Focuses: rest-api, privacy Cc:

Description

Via endpoints like /wp/v2/comments?search=$term, it's currently possible to perform email discovery through brute force. In this case, emails of commenters.

Not exactly the same, but previous discussion (for login forms) is at: https://core.trac.wordpress.org/ticket/9568#comment:82

After an H1 report and some discussion within the security team, it was decided we should probably "fix" this and have more public discussion.

Authorized users should be able to search comment data that's non-public.

Report by dawidpieper.

Change History (10)

#1 @dd32
2 months ago

  • Owner dd32 deleted

Just removing my assignment from the ticket, as I don't have any work underway on this and don't want to risk someone not touching it because of the assignment :)

#2 @waike
7 weeks ago

Related issue: Using the search parameter on /wp-json/wp/v2/users, it's possible to extract user email addresses one character at a time, because the search parameter is matched against users' email fields.

https://wordpress.org/wp-json/wp/v2/users?search=m@ottodestruct.com     -> no results
https://wordpress.org/wp-json/wp/v2/users?search=n@ottodestruct.com     -> no results
https://wordpress.org/wp-json/wp/v2/users?search=o@ottodestruct.com     -> 1 result
(...)
https://wordpress.org/wp-json/wp/v2/users?search=mtto@ottodestruct.com  -> no results
https://wordpress.org/wp-json/wp/v2/users?search=ntto@ottodestruct.com  -> no results
https://wordpress.org/wp-json/wp/v2/users?search=otto@ottodestruct.com  -> 1 result

I wrote a quick proof-of-concept tool to automate this process.

Link: https://github.com/JeppW/WordPress-Email-Extractor

This ticket was mentioned in PR #1660 on WordPress/wordpress-develop by fictiont.


3 weeks ago

  • Keywords has-patch added; needs-patch removed

There is a security problem in the current WP REST API.

Using search parameter available at /wp-json/wp/v2/users and /wp-json/wp/v2/comments it is possible to brute force users emails and comment authors emails as shown on example below provided by @JeppW:

https://wordpress.org/wp-json/wp/v2/users?search=m@ottodestruct.com -> no results
https://wordpress.org/wp-json/wp/v2/users?search=n@ottodestruct.com -> no results
https://wordpress.org/wp-json/wp/v2/users?search=o@ottodestruct.com -> 1 result
(...)
https://wordpress.org/wp-json/wp/v2/users?search=mtto@ottodestruct.com -> no results
https://wordpress.org/wp-json/wp/v2/users?search=ntto@ottodestruct.com -> no results
https://wordpress.org/wp-json/wp/v2/users?search=otto@ottodestruct.com -> 1 result

User email and comment author email aren't public fields, they aren't returned in the users list and comments list.
This patch fixes problem by removing fields user_email and comment_author_email from search logic at related endpoints:
../v2/users?search=
../v2/comments?search=

Here is an example from a local machine of how the fix works:

  • before applying the fix:

https://i0.wp.com/user-images.githubusercontent.com/4171817/132241360-dd98d6c6-3bf2-4184-a845-8c252e1ddc26.png

  • after applying the fix:

https://i0.wp.com/user-images.githubusercontent.com/4171817/132241373-beca1ec9-6353-4b2f-ab76-11569f96991a.png

Props ehtis, waike.
Fixes https://core.trac.wordpress.org/ticket/53784.

#4 @fictiont
3 weeks ago

  • Component changed from Users to REST API
  • Keywords has-unit-tests has-screenshots added

#5 @fictiont
3 weeks ago

  • Focuses privacy added

#6 follow-up: @audrasjb
3 weeks ago

Thank for working on this @fictiont !
The PR looks good to me, I'm only wondering if the long comment above the change is really needed.
I'd prefer to add a hook to filter $allowed_columns and to use it to document properly the default behavior.

#7 in reply to: ↑ 6 @fictiont
3 weeks ago

Replying to audrasjb:

Thank for working on this @fictiont !
The PR looks good to me, I'm only wondering if the long comment above the change is really needed.
I'd prefer to add a hook to filter $allowed_columns and to use it to document properly the default behavior.

Thank you @audrasjb for the suggestion! I agree that would be a better approach. However, I'm not sure I could make an update to the patch before the meeting.

#8 @audrasjb
3 weeks ago

No worries, that's not a blocker at all :)

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


3 weeks ago

#10 @audrasjb
3 weeks ago

  • Keywords dev-feedback added
  • Milestone changed from Future Release to 5.9

Marking for 5.9 consideration

Note: See TracTickets for help on using tickets.