Skip to content

Users: Prevent non-numeric searches from matching user IDs in WP_User_Query. - #15262

Open
abhishekfdd wants to merge 1 commit into
WordPress:trunkfrom
abhishekfdd:fix/57731-user-query-id-search-non-numeric
Open

abhishekfdd wants to merge 1 commit into
WordPress:trunkfrom
abhishekfdd:fix/57731-user-query-id-search-non-numeric

Conversation

@abhishekfdd

Copy link
Copy Markdown

When ID is in search_columns, WP_User_Query::get_search_sql() builds ID = %s with the raw search term. MySQL casts the string to a number for that comparison, using only its leading digits, so '123abc@email.com' becomes 123. As a result, an email lookup such as:

new WP_User_Query( array(
	'search'         => '123abc@email.com',
	'search_columns' => array( 'user_email', 'ID' ),
) );

returns user 123 even when no user has that email address. Code that uses this to resolve an email to an account can silently get a different user.

Change

  • The ID comparison is only added when the search term (after whitespace and the * wildcards are trimmed in prepare_query()) is a plain non-negative integer string, checked with ctype_digit(). is_numeric() is not used because it accepts 1e3, 12.5, -5, +5 and whitespace-padded values, all of which MySQL would also cast.
  • If no search clauses remain (only ID was requested and the term is not an integer), the method returns AND 1=0, so the query matches no users. It does not drop the search clause, which would return every user.
  • The column auto-detection used when search_columns is not passed is unchanged. Note that for terms like 1e3 or 5.0, which pass is_numeric(), auto-detection still selects user_login and ID, but ID now no longer matches them through MySQL's cast.

SQL before

WHERE 1=1 AND (user_email LIKE '1abc@example.com' OR ID = '1abc@example.com')  -- returns user 1
WHERE 1=1 AND (ID = '1abc@example.com')                                         -- returns user 1

SQL after

WHERE 1=1 AND (user_email LIKE '1abc@example.com')  -- returns no users
WHERE 1=1 AND 1=0                                   -- returns no users
WHERE 1=1 AND (ID = '1')                            -- unchanged for integer searches

Leading zeros (0123)

0123 is still treated as ID 123. It is an unambiguous integer with no other meaning in a digits-only string, and this is the behaviour MySQL already gives (ID = '0123'). Rejecting it would be a behaviour change that does not fix any mismatch. The value is still bound with %s, so a very long digit string never overflows to PHP_INT_MAX the way an (int) cast or %d would.

Side effect

If a user_search_columns filter returns an empty array, the method previously built AND (), which caused a database error. It now builds AND 1=0. The result is the same (no users), without the error.

Testing

Automated tests: npm run test:php -- --group 57731. These cover:

  • the reported case: a non-numeric search with ['user_email', 'ID'] returns no users
  • the same search still finds the user whose email matches, and not the user whose ID is the prefix
  • ID only with a non-numeric search returns no users (not all users)
  • an integer search with ['ID'] matches, including 0-prefixed and * wildcard variants
  • the email search with leading, trailing and both * wildcards
  • %de0, %d.0, %d.5, -%d and +%d inputs (built from the user's real ID) do not match by ID. The e0, .0 and + variants do match on trunk.
  • the numeric search with no search_columns (auto-detection) still matches by ID

21 of the 27 new tests fail on trunk. The full user group passes on both single site and multisite.

Manual testing:

  1. Create a user and note their ID, e.g. 1.
  2. wp eval 'var_dump( ( new WP_User_Query( array( "search" => "1abc@example.com", "search_columns" => array( "user_email", "ID" ), "fields" => "ID" ) ) )->get_results() );'
  3. Before: array(1) { [0]=> string(1) "1" }. After: array(0) {}.

Sibling query classes (WP_Comment_Query, WP_Site_Query, WP_Network_Query) were checked. They only use LIKE in get_search_sql(), so they are not affected.

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

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Reproducing the bug, implementing the fix and tests, and drafting this description, following my step-by-step instructions.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

…_Query.

When `ID` was included in `search_columns`, the search term was compared using `ID = %s`. MySQL casts the string to a number for that comparison, so a search such as `123abc@example.com` matched the user with ID 123.

The ID column is now only compared when the search term is a plain non-negative integer string. If no other columns remain to be searched, an always-false condition is used so that no users are returned, rather than all users.

Includes unit tests.

See #57731.
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props abhishekfdd.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant