Make WordPress Core

Opened 4 months ago

Closed 2 weeks ago

#65051 closed defect (bug) (fixed)

$_REQUEST['term'] used unsanitized in user search query

Reported by: rajeshcp Owned by: westonruter
Priority: normal Milestone: 7.1
Component: Networks and Sites Version: 3.4
Severity: minor Keywords: has-patch needs-testing has-test-info has-unit-tests has-screenshots
Cc: Focuses: multisite, coding-standards

Description

User-supplied search term is concatenated directly into the get_users() search argument without

sanitize_text_field() or wp_unslash().

Attachments (2)

test-65051-sanitize.png (48.2 KB ) - added by liaison 4 months ago.
test-65051-sanitize
email-with-apostrophe.png (105.8 KB ) - added by westonruter 2 weeks ago.

Download all attachments as: .zip

Change History (17)

This ticket was mentioned in PR #11530 on WordPress/wordpress-develop by rajeshcpr.


4 months ago
#1

User-supplied search term is concatenated directly into the get_users() search argument without

sanitize_text_field() or wp_unslash().

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

Fixes #65051

## Use of AI Tools

#2 @gaurangsondagar
4 months ago

  • Focuses coding-standards added

Tested the patch and confirmed the issue: https://github.com/WordPress/wordpress-develop/pull/11530/commits/5eed1c8ea50eb3dfda7605749f267bf9e3234dc3

Environment:

  • WordPress: 7.1-alpha-62161-src
  • PHP: 8.3.30
  • Browser: Chrome
  • Database: MySQL 8.4.8
  • OS: Ubuntu

1) The current implementation uses $_REQUEST[term] directly without sanitization.
2) The patch correctly applies 'wp_unslash()' and 'sanitize_text_field()', which aligns with WordPress data handling standards.
3) Verified that the user search functionality continues to work as expected after the change.

This is a valid security improvement and works as expected

Screenshot for reference: https://kommodo.ai/i/s2Bol19v4cwB50UNttQp

#3 @audrasjb
4 months ago

  • Component GeneralNetworks and Sites
  • Focuses multisite added
  • Milestone Awaiting Review7.1
  • Severity majornormal
  • Version trunk

#4 @liaison
4 months ago

Test Report & Security Impact Analysis

  1. Environment

WordPress: 7.1-alpha (trunk)

Setup: Multisite Network Admin

Verification Method: Standalone PHP Mock + Browser DevTools Interception

  1. Security Analysis: Why a PoC is currently "Silent"

I would like to clarify the current impact of this vulnerability. While the fix is essential, a successful XSS alert is currently unlikely to trigger in the default UI for the following reasons:

Empty Response on Malicious Payload: When a payload like <script>alert('XSS')</script> is injected into the term parameter, the backend executes a get_users query. Since no user account exists with such a string, the database returns an empty set. The resulting Ajax response is an empty array [].

Safe Client-Side Handling: The current core JavaScript expects a JSON array of user objects. When it receives [], it simply does not render any dropdown items, preventing the payload from being echoed back into the DOM.

  1. Why the fix is still critical

Log Poisoning / Second-Order XSS: If the search term is recorded in server-side logs (e.g., Audit logs or Slow Query logs) viewed in other admin interfaces, the script could execute.

Future-Proofing: Any UI changes displaying "No results found for: [term]" would immediately turn this into a high-risk Reflected XSS.

Data Integrity: Aligning with WP coding standards by sanitizing at the entry point.

  1. Verification Results

[Next comment]My standalone tests confirm that the patch correctly strips HTML tags at the entry point.

Payload Intercepted via DevTools:

action: autocomplete-user
term: <script>alert('XSS')</script>Admin
site_id: 2
Backend Observation (using attached test script):

Before Patch: get_users receives the raw <script> tag.

After Patch: get_users receives the sanitized string alert('XSS')Admin.

test-65051-sanitize

Last edited 4 months ago by liaison (previous) (diff)

@liaison
4 months ago

test-65051-sanitize

#5 @liaison
4 months ago

Test Report
Ticket: #65051 - $_REQUEST[\'term\'] used unsanitized in user search query

Environment
WordPress Version: 7.1-alpha (trunk)

PHP Version: 8.x

Test Method: Standalone Mock / Integration Test

OS: Windows (MINGW64)

Testing Methodology
I performed a deep-dive verification using a standalone mock script to isolate the data flow within wp_ajax_autocomplete_user(). By stubbing the core dependencies (is_multisite, get_users, etc.), I was able to intercept the exact arguments being passed to the user query logic.

Test Results

  1. Confirming the Vulnerability (Before Patch)

Using a malicious payload: <script>alert('hack')</script>Admin
The input was passed directly to the search argument without any sanitization.

Intercepted Query: [Intercepted] get_users() called with 'search' => '<script>alert(\'hack\')</script>Admin'

Status: ❌ Confirmed. Raw HTML/Script tags reached the query level.

  1. Verification of Fix (After Patch)

Applied sanitize_text_field( wp_unslash( ... ) ) to the $term variable.

Intercepted Query: [Intercepted] get_users() called with 'search' => '*alert(\'hack\')Admin*'

Status: ✅ Fixed. The <script> tags were successfully stripped before reaching get_users().

Execution Log Output (before patch)

--- Running Standalone Invocation Test ---

--- [Demo before patch] ---
[Intercepted] get_users() called with 'search' => '<script>alert(\'hack\')</scri p
t>Admin'
[JSON Response]: {"success":true}

--- [Demo after patch] ---
[Intercepted] get_users() called with 'search' => 'alert(\'hack\')Admin'
[JSON Response]: {"success":true}

--- [Test Target: wp_ajax_autocomplete_user] ---
[Intercepted] get_users() called with 'search' => '*<script>alert(\'hack\')</sc ipt>Admin*'

[wp_die] Value: []

Execution Log Output (after patch)

Plaintext
--- Running Standalone Invocation Test ---

--- [Demo before patch] ---
[Intercepted] get_users() called with 'search' => '<script>alert(\'hack\')</script>Admin'
[JSON Response]: {"success":true}

--- [Demo after patch] ---
[Intercepted] get_users() called with 'search' => 'alert(\'hack\')Admin'
[JSON Response]: {"success":true}

--- [Test Target: wp_ajax_autocomplete_user] ---
[Intercepted] get_users() called with 'search' => '*alert(\'hack\')Admin*'
[wp_die] Value: []

Verdict
The patch effectively resolves the issue by sanitizing the user-supplied search term. It prevents potential XSS payloads from being processed in the backend logic while maintaining the expected search functionality.

test-65051-sanitize.php

<?php
/**
 * Standalone Test: Invoking wp_ajax_autocomplete_user directly
 */

define( 'DOING_AJAX', true );
define( 'WP_ADMIN', true );

// fake $wp_db 
$GLOBALS['wpdb'] = unserialize('O:8:"stdClass":0:{}'); 

// --- Stub Functions (to pass test)---
function auth_redirect() {} 
function check_ajax_referer( $action ) { return true; } 
function current_user_can( $cap ) { return true; } 

function wp_unslash( $data ) { return stripslashes( $data ); }
function sanitize_text_field( $str ) { return strip_tags( $str ); }

function get_users( $args ) {
    $search_term = $args['search'] ?? '(no search term)';

    if ( '(no search term)' !== $search_term ) {
        echo "[Intercepted] get_users() called with 'search' => " . var_export($search_term, true) . "\n";
    }
    
    return array(); 
}

function wp_send_json( $response ) {
    echo "[JSON Response]: " . json_encode( $response ) . "\n";
}

function wp_die( $msg = '' ) { 
    echo "\n[wp_die] Value: $msg\n";
}

if ( ! function_exists( 'is_multisite' ) ) {
    function is_multisite() {
        return true; 
    }
}

if ( ! function_exists( 'wp_is_large_network' ) ) {
    function wp_is_large_network() {
        return false; 
    }
}

if ( ! function_exists( 'get_current_blog_id' ) ) {
   function get_current_blog_id() {
        return 1;
    }
}


if ( ! function_exists( 'wp_json_encode' ) ) {
   function wp_json_encode( $data ) {
        return json_encode( $data );
    }
}

if ( ! function_exists( 'get_current_screen' ) ) {
    function get_current_screen() { return null; }
}

if ( ! function_exists( 'wp_parse_args' ) ) {
    function wp_parse_args( $args, $defaults = array() ) {
        return array_merge( $defaults, $args );
    }
}

// --- target source ---
require_once 'wp-admin/includes/ajax-actions.php'; 

/**
 * Simulation Demo
 */

function wp_ajax_autocomplete_user_demo_before_patch() {
    $term = $_REQUEST['term']; 
 
    get_users( array(
        'search' => $term,
        'fields' => array( 'ID', 'user_login' ),
    ) );

    wp_send_json( array( 'success' => true ) );
}

function wp_ajax_autocomplete_user_demo_after_patch() {
    $term = sanitize_text_field( wp_unslash( $_REQUEST['term'] ) );

    get_users( array(
        'search' => $term,
        'fields' => array( 'ID', 'user_login' ),
    ) );

    wp_send_json( array( 'success' => true ) );
}


echo "--- Running Standalone Invocation Test ---\n";

$_REQUEST['term'] = "<script>alert('hack')</script>Admin";

echo "\n--- [Demo before patch] ---\n";
try {
    wp_ajax_autocomplete_user_demo_before_patch();
} catch (Exception $e) {
    echo "Caught: " . $e->getMessage();
}

echo "\n--- [Demo after patch] ---\n";
try {
    wp_ajax_autocomplete_user_demo_after_patch();
} catch (Exception $e) {
    echo "Caught: " . $e->getMessage();
}

echo "\n--- [Test Target: wp_ajax_autocomplete_user] ---\n";
try {
    wp_ajax_autocomplete_user();
} catch (Exception $e) {
    echo "Caught: " . $e->getMessage();
}

#6 @liaison
4 months ago

  • Keywords has-test-info added

#7 @liaison
4 months ago

  • Keywords has-unit-tests added

This ticket was mentioned in Slack in #core-test by r1k0. View the logs.


3 months ago

#9 @vgnavada
3 months ago

  • Keywords has-screenshots added

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/11530

Environment

  • WordPress: 7.1-alpha-20260409.114541
  • PHP: 8.3.30
  • Server: PHP.wasm
  • Database: WP_SQLite_Driver (Server: 8.0.38 / Client: 3.51.0)
  • Browser: Chrome 148.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.4
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.1
  • Testing setup: WordPress Playground with Multisite enabled

Actual Results

  1. ✅ Patch works as expected based on functional Playground testing.

Test 1: Malicious payload through browser console

Tested the autocomplete-user AJAX action using the following payload:

<script>alert('XSS')</script>Admin

The request completed successfully and returned an empty array:

[] success

No alert was triggered, no malicious autocomplete suggestion was rendered, and no visible console error was observed.

https://i.ibb.co/679vhfqr/image.png

Test 2: Normal term through the browser console

Tested the same autocomplete-user AJAX action using a normal term:

admin

with autocomplete_type set to add.

The request completed successfully and returned an empty array:

[] success

This is not treated as a failure because, in the Playground setup, the admin user is already part of site_id: 1, so the add-user autocomplete flow may exclude that user from the result.

https://i.ibb.co/CsV780c2/image.png

Test 3: Manual UI input test
Tested the payload directly in the Network Admin user autocomplete input field:

<script>alert('XSS')</script>Admin

The field accepted the typed value, which is expected for a text input. No alert was triggered, no script was executed, no malicious autocomplete suggestion was rendered, and the page did not break.

https://i.ibb.co/GQKvvYHN/image.png

Additional Notes

  • Tested using WordPress Playground with PR: 11530 and Multisite enabled.
  • The first console screenshot shows the malicious payload test returning [] success.
  • The second console screenshot shows the normal admin term test returning [] success.
  • This Playground test confirms the visible browser and AJAX behavior.
  • Based on the Playground test, the patch does not visibly break the autocomplete-user AJAX action, and the malicious payload does not execute or render in the browser.

#10 @saadtajik
5 weeks ago

Test Report: Network Payload & AJAX Verification

Environment:

Local Multisite Network (trunk)

Method: Monitored XHR/AJAX requests via Browser Developer Tools during the "Add Existing User" workflow.

Testing Steps & Results:

  1. Vulnerability Confirmation (Before Patch):

I entered the payload <script>alert('XSS')</script>Admin into the Username field to trigger the autocomplete-user AJAX action.

Intercepted Request Payload: term: <script>alert('XSS')</script>Admin

Result: The Network tab confirmed that raw HTML was being passed directly to the backend and into the get_users() query without any stripping.

  1. Fix Verification (After PR #11530):

I applied the proposed patch and repeated the exact same AJAX request from the UI.

Backend Processing Check: The implementation of sanitize_text_field( wp_unslash( $_REQUESTterm ) ) successfully intercepted the payload.

Result: The script tags were completely stripped on the server side before the string was passed to the get_users() query arguments.

Verdict: ✅ Issue Resolved.
The expected UI behavior remains perfectly intact, but the backend data flow is now strictly sanitized. This cleanly patches the raw input vulnerability without introducing any performance regressions or PHP warnings in the debug log.

@wildworks commented on PR #11673:


3 weeks ago
#11

Thanks for the PR, but let me close this in favor of #11530. I plan to port the unit tests there.

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


2 weeks ago

#13 @westonruter
2 weeks ago

  • Owner changed from rajeshcp to westonruter
  • Status assignedreviewing

#14 @westonruter
2 weeks ago

  • Severity normalminor
  • Version3.4

This is not a security vulnerability.

The only problem being fixed here is an extremely narrow case where you could fail to find a user when the email contains an apostrophe (which I didn't even know was allowed).

Surprisingly, I am able to create a user with the email address bat'leth.qapla'@klingon.example.com, as seen in email-with-apostrophe.png.

When I try adding a user via autocomplete using that email address, I get no auto-completion. This is because the search is being made with the apostrophes being slashed: bat\'leth.qapla\'@klingon.example.com.

The search works successfully when the input var is correctly unslashed.

That said, it's not clear to me whether most email providers even accepted, but I was able to to successfully send myself from one Gmail address to another with +bat'leth added to the username.

#15 @westonruter
2 weeks ago

  • Resolutionfixed
  • Status reviewingclosed

In 63016:

Networks and Sites: Improve user autocomplete search term handling.

In wp_ajax_autocomplete_user(), unslash and sanitize the term request parameter before it is passed to get_users(). Unslashing fixes searching for an email address containing an apostrophe (valid per is_email()), which could previously never match because wp_magic_quotes() added a slash which wpdb::esc_like() then escaped as a literal. Note that the raw term was already safely handled in the user query, since WP_User_Query passes the search term through wpdb::prepare(), so this is a hardening and correctness fix rather than a security fix.

Additionally, a missing, non-string, or empty term now short-circuits with a 0 response instead of returning an empty array, avoiding a PHP warning and needless user queries. Asterisks are also trimmed from the term given that wildcards are appended to it; a term consisting only of asterisks previously resulted in an empty search which matched all users on the network.

Also introduce the Tests_Ajax_wpAjaxAutocompleteUser test class covering the Ajax action's search behavior, input handling, and capability checks.

Developed in https://github.com/WordPress/wordpress-develop/pull/11530.
Follow-up to r19897, r20279.

Props rajeshcp, wildworks, westonruter, liaison, gaurangsondagar, vgnavada, saadtajik.
Fixes #65051.

Note: See TracTickets for help on using tickets.