Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhishekfdd wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/wp-includes/class-wp-user-query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,12 +948,23 @@ protected function get_search_sql( $search, $columns, $wild = false ) {

foreach ( $columns as $column ) {
if ( 'ID' === $column ) {
$searches[] = $wpdb->prepare( "$column = %s", $search );
/*
* Only compare against the ID column when the search is a plain non-negative integer.
* Otherwise MySQL casts the string to a number, so '123abc' would match user 123.
*/
if ( ctype_digit( (string) $search ) ) {
$searches[] = $wpdb->prepare( "$column = %s", $search );
}
} else {
$searches[] = $wpdb->prepare( "$column LIKE %s", $like );
}
}

// No searchable columns remain, e.g. only 'ID' was requested with a non-numeric search.
if ( ! $searches ) {
return ' AND 1=0';
}

return ' AND (' . implode( ' OR ', $searches ) . ')';
}

Expand Down
159 changes: 159 additions & 0 deletions tests/phpunit/tests/user/query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2405,4 +2405,163 @@ public function test_query_does_not_have_leading_whitespace() {

$this->assertSame( ltrim( $q->request ), $q->request, 'The query has leading whitespace' );
}

/**
* @ticket 57731
*/
public function test_search_with_id_column_should_not_match_non_numeric_search_starting_with_user_id() {
$user_id = self::factory()->user->create();

$q = new WP_User_Query(
array(
'search' => $user_id . 'abc@example.com',
'search_columns' => array( 'user_email', 'ID' ),
'fields' => 'ID',
)
);

$this->assertSame( array(), $q->get_results() );
}

/**
* @ticket 57731
*/
public function test_search_with_id_column_should_match_email_starting_with_another_user_id() {
$user_id = self::factory()->user->create();
$email_user_id = self::factory()->user->create(
array(
'user_email' => $user_id . 'abc@example.com',
)
);

$q = new WP_User_Query(
array(
'search' => $user_id . 'abc@example.com',
'search_columns' => array( 'user_email', 'ID' ),
'fields' => 'ID',
)
);

$this->assertSame( array( (string) $email_user_id ), $q->get_results() );
}

/**
* @ticket 57731
*/
public function test_search_with_only_id_column_and_non_numeric_search_should_match_no_users() {
$user_id = self::factory()->user->create();

$q = new WP_User_Query(
array(
'search' => $user_id . 'abc',
'search_columns' => array( 'ID' ),
'fields' => 'ID',
)
);

$this->assertSame( array(), $q->get_results(), 'No users should be returned.' );
$this->assertStringContainsString( 'AND 1=0', $q->request, 'The query should contain an always-false condition.' );
}

/**
* @ticket 57731
*
* @dataProvider data_search_with_id_column_should_match_integer_search
*
* @param string $format Search string format, where %d is replaced with the user ID.
*/
public function test_search_with_id_column_should_match_integer_search( $format ) {
$user_id = self::factory()->user->create();

$q = new WP_User_Query(
array(
'search' => sprintf( $format, $user_id ),
'search_columns' => array( 'ID' ),
'fields' => 'ID',
)
);

$this->assertSame( array( (string) $user_id ), $q->get_results() );
}

/**
* Data provider.
*
* @return array[]
*/
public static function data_search_with_id_column_should_match_integer_search() {
return array(
'plain integer' => array( '%d' ),
'leading zero' => array( '0%d' ),
'leading wildcard' => array( '*%d' ),
'trailing wildcard' => array( '%d*' ),
'both wildcards' => array( '*%d*' ),
);
}

/**
* @ticket 57731
*
* @dataProvider data_search_with_id_column_should_not_match_non_integer_search
*
* @param string $format Search string format, where %d is replaced with the user ID.
* @param string[] $search_columns Columns to search.
*/
public function test_search_with_id_column_should_not_match_non_integer_search( $format, $search_columns ) {
$user_id = self::factory()->user->create();

$q = new WP_User_Query(
array(
'search' => sprintf( $format, $user_id ),
'search_columns' => $search_columns,
'fields' => 'ID',
)
);

$this->assertSame( array(), $q->get_results(), 'No users should be returned.' );
$this->assertStringNotContainsString( "ID = '", $q->request, 'The ID column should not be searched.' );
}

/**
* Data provider.
*
* @return array[]
*/
public static function data_search_with_id_column_should_not_match_non_integer_search() {
$formats = array(
'email' => '%dabc@example.com',
'email with leading wildcard' => '*%dabc@example.com',
'email with trailing wildcard' => '%dabc@example.com*',
'email with both wildcards' => '*%dabc@example.com*',
'exponent' => '%de0',
'decimal with zero fraction' => '%d.0',
'decimal' => '%d.5',
'negative' => '-%d',
'explicit positive sign' => '+%d',
);

$data = array();
foreach ( $formats as $name => $format ) {
$data[ "$name, ID only" ] = array( $format, array( 'ID' ) );
$data[ "$name, user_email and ID" ] = array( $format, array( 'user_email', 'ID' ) );
}

return $data;
}

/**
* @ticket 57731
*/
public function test_search_without_search_columns_should_match_numeric_search_by_id() {
$user_id = self::factory()->user->create();

$q = new WP_User_Query(
array(
'search' => (string) $user_id,
'fields' => 'ID',
)
);

$this->assertContains( (string) $user_id, $q->get_results() );
}
}
Loading