WordPress 4.8.2 SQLi vulnerability

On 31th Oct, WordPress 4.8.3 has been released. This is a security release for all previous versions and we strongly encourage you to update your sites immediately.

Details

WordPress versions 4.8.2 and earlier are affected by an issue where $wpdb->prepare() can create unexpected and unsafe queries leading to potential SQL injection (SQLi). WordPress core is not directly vulnerable to this issue, but we’ve added hardening to prevent plugins and themes from accidentally causing a vulnerability.

Analysis

The following $wpdb wording is affected by this vulnerability:

$where = $wpdb->prepare(" WHERE foo = %s", $_GET['data']);

$query = $wpdb->prepare("SELECT * FROM something $where LIMIT %d, %d", 1, 2);

This is known as “double-preparing” and is not a good design.

Also, don’t do this:

$where = "WHERE foo = '" . esc_sql($_GET['data']) . "'";

$query = $wpdb->prepare("SELECT * FROM something $where LIMIT %d, %d", 1, 2);

The query and parameters should be built separately and then executed by the prepare method.

<p style="text-indent: 2em;">$where = "WHERE foo = %s";<br>$args = [$_GET['data']];<br>$args[] = 1;<br>$args[] = 2;<br>$query = $wpdb->prepare("SELECT * FROM something $where LIMIT %d, %d", $args);<br></p>

 

In order to understand the easy-to-attack code for $ wpdb in this report, let’s follow the analysis of WordPress’s internal section on WPDB::prepare. First look at the source code
public function prepare( $query, $args ) {

if ( is_null( $query ) )
return;
// This is not meant to be foolproof -- but it will catch obviously incorrect usage.
if ( strpos( $query, '%' ) === false ) {
_doing_it_wrong( 'wpdb::prepare', sprintf( __( 'The query argument of %s must have a placeholder.' ), 'wpdb::prepare()' ), '3.9.0' );
}
$args = func_get_args();
array_shift( $args );
// If args were passed as an array (as in vsprintf), move them up
if ( isset( $args[0] ) && is_array($args[0]) )
$args = $args[0];
$query = str_replace( "'%s'", '%s', $query ); // in case someone mistakenly already singlequoted it
$query = str_replace( '"%s"', '%s', $query ); // doublequote unquoting
$query = preg_replace( '|(?<!%)%f|' , '%F', $query ); // Force floats to be locale unaware
$query = preg_replace( '|(?<!%)%s|', "'%s'", $query ); // quote the strings, avoiding escaped strings like %%s
array_walk( $args, array( $this, 'escape_by_ref' ) );
return @vsprintf( $query, $args );
}

First, it uses vsprintf (which is basically identical to sprintf) to replace placeholders with values.

Second, it uses str_replace to quote placeholders properly (even unquoting first to prevent double quotes).

Third, if passed a single argument and that argument is an array, then it will replace the arguments with the value of that array. Meaning that calling $wpdb->prepare($sql, [1, 2]) is identical to calling $wpdb->prepare($sql, 1, 2). This will be important later.

Look at code

$items = implode(", ", array_map([$wpdb, '_real_escape'], $_GET['items']));

$sql = "SELECT * FROM foo WHERE bar IN ($items) AND baz = %s";
$query = $wpdb->prepare($sql, $_GET['baz']);
Here is a small feature related to vsprintf
vsprintf('%s, %d, %s', ["a", 1, "b"]); // "a, 1, b"

vsprintf('%s, %d, %1$s', ["a", 2, "b"]); // "a, 2, a"
Note that %n$s does not read the next argument, but instead reads the n-th position. We can be based on the verification of the above query, assuming that we send the following request information to the server
$_GET['items'] = ['%1$s'];

$_GET['baz'] = "test";

Now, the query will be changed to SELECT * FROM foo WHERE bar IN (‘test’) AND baz = ‘test’; but this changes the meaning of the query, in order to more clearly explain the sql injection, you can construct such a request data

$_GET['items'] = ['%1$c) OR 1 = 1 /*'];

$_GET['baz'] = 39;
It should be noted that: sprintf also accept another type of parameter: %c its role is like chr() to convert the decimal number into a character. Where ASCII table 39 is the (single quotation mark) ASCII code. Then the entire query execution sql statement is:
SELECT * FROM foo WHERE bar IN ('') OR 1 = 1 /*' AND baz = 'test';

It turns out that the flaw also exists in WordPress core file /wp-includes/meta.php.

if ( $delete_all ) {

$value_clause = '';
if ( '' !== $meta_value && null !== $meta_value && false !== $meta_value ) {
$value_clause = $wpdb->prepare( " AND meta_value = %s", $meta_value );
}
$object_ids = $wpdb->get_col( $wpdb->prepare( "SELECT $type_column FROM $table WHERE meta_key = %s $value_clause", $meta_key ) );
}

Reference: