WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 2 months ago

#53784 assigned defect (bug)

Limiting user enumeration through the REST API

Reported by: ehtis Owned by:
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch has-unit-tests has-screenshots dev-reviewed
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 (13)

#1 @dd32
6 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
6 months 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.


5 months 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
5 months ago

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

#5 @fictiont
5 months ago

  • Focuses privacy added

#6 follow-up: @audrasjb
5 months 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
5 months 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
5 months ago

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

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


5 months ago

#10 @audrasjb
5 months ago

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

Marking for 5.9 consideration

#11 @TimothyBlynJacobs
3 months ago

  • Keywords dev-reviewed added; dev-feedback removed

I've left some feedback on the PR. I think this still needs some more work before it is ready. We may also want to commit this in a minor release so it can be backported. Cc: @peterwilsoncc.

This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.


3 months ago

#13 @hellofromTonya
2 months ago

  • Milestone changed from 5.9 to 6.0

5.9 Beta 1 is tomorrow. As there's still work to do in the PR, moving it to 6.0. However, if it should land in a minor for backporting, please move it to that minor milestone.

Note: See TracTickets for help on using tickets.