Limiting user enumeration through the REST API

[ehtis]

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.

[dd32]

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 :)

[waike]

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

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:

• after applying the fix:

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

[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.

[fictiont]

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.

[audrasjb]

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

[audrasjb]

Marking for 5.9 consideration

[TimothyBlynJacobs]

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.

[hellofromTonya]

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.

I've merged in and retargeted this to trunk (new default branch name).

So I think there are valid multiple points of view here, but from my perspective we should move the current_user_can check outside of WP_Comments_Query and into the Controller so it functions the same way as WP_User_Query.

I think the Query class should be responsible for providing the data the programmer asked for, and for the individual interfaces to handle access controls how they deem appropriate. Otherwise I can't create an interface for let's say comment moderation that has those more advanced order by support unless I also grant list_users which may be undesirable.

Our User, Term and Comment Query APIs are currently unified in not performing capability checks themselves. This is of course in contrast to WP_Query itself which is why I think my POV is debatable. However, I think WP_Query is an older design and itself mixes concerns by also being the main query handler. If possible, we should try and maintain the design pattern that the new query classes currently follow.

I'm inclined to agree with @TimothyBJacobs.

The interface (i.e. the controller) should determine how it will respond to a request. That comes with the responsibility to ensure that it doesn't expose information that it shouldn't. One interface may allow access to information in some or all circumstances, and another may lock it down entirely.

The purpose of WP_Comment_Query is to provide a foundation for querying comments. WP_Comment_Query cannot determine the purpose of a particular interface, the use cases that the interface is intended to cover, and cannot make this a blanket decision for all interfaces that query comments.

For all intents and purposes, if ( current_user_can( 'list_users' ) ) {} is akin to a can_include_author_email() method. Not all interfaces will need or use this, thus it could be argued that adding the capability checks to WP_Comment_Query would verge on violating the Interface Segregation Principle.

---

An outstanding use case: If I wanted to have a frontend interface for comment moderation that utilized the REST API and allowed sorting by the comment author's email address, I would still need to grant the list_users capability, which as @TimothyBJacobs highlighted, may not be desirable.

This use case goes outside of this ticket IMO, and may mean adding new list_user_email and list_user_ip capabilities - noting that desiring access to one may not mean desiring access to the other.

Thanks both, let's go with that approach then.

It's unlikely I'll have any bandwidth to push any updates to the PR prior to the WP beta next week.

[costdev]

With 6.0 RC1 tomorrow, this ticket still requires more work. Moving to the Future Release milestone.

Looks like someone else reported this issue and it was patched in WordPress 6.3.2.

Reference: ​wpscan.com/blog/email-leak-oracle-vulnerability-addressed-in-wordpress-6-3-2

[ironprogrammer]

For clarification regarding the security update to 6.3.2 in comment:19, that fix addresses the case where the search param is used directly on the Users endpoint.

However, as originally reported, a search query against the Comments endpoint remains vulnerable to oracle-style attacks, e.g. /wp-json/wp/v2/comments?search=@, to discover commenter emails. This ticket/concern remains valid IMHO.