Skip to content

Commit a2693fd

Browse files
committed
Database: Restore numbered placeholders in wpdb::prepare().
[41496] removed support for numbered placeholders in queries send through `wpdb::prepare()`, which, despite being undocumented, were quite commonly used. This change restores support for numbered placeholders (as well as a subset of placeholder formatting), while also adding extra checks to ensure the correct number of arguments are being passed to `wpdb::prepare()`, given the number of placeholders. See #41925. Built from https://develop.svn.wordpress.org/trunk@42056 git-svn-id: http://core.svn.wordpress.org/trunk@41885 1a063a9b-81f0-0310-95a4-ce76da25c4cd
1 parent 6f2db8e commit a2693fd

File tree

5 files changed

+142
-41
lines changed

5 files changed

+142
-41
lines changed

Diff for: wp-includes/formatting.php

+5
Original file line numberDiff line numberDiff line change
@@ -3754,6 +3754,11 @@ function _deep_replace( $search, $subject ) {
37543754
* Sometimes, spot-escaping is required or useful. One example
37553755
* is preparing an array for use in an IN clause.
37563756
*
3757+
* NOTE: Since 4.8.3, '%' characters will be replaced with a placeholder string,
3758+
* this prevents certain SQLi attacks from taking place. This change in behaviour
3759+
* may cause issues for code that expects the return value of esc_sql() to be useable
3760+
* for other purposes.
3761+
*
37573762
* @since 2.8.0
37583763
*
37593764
* @global wpdb $wpdb WordPress database abstraction object.

Diff for: wp-includes/meta.php

+3-4
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,11 @@ function delete_metadata($meta_type, $object_id, $meta_key, $meta_value = '', $d
364364
return false;
365365

366366
if ( $delete_all ) {
367-
$value_clause = '';
368367
if ( '' !== $meta_value && null !== $meta_value && false !== $meta_value ) {
369-
$value_clause = $wpdb->prepare( " AND meta_value = %s", $meta_value );
368+
$object_ids = $wpdb->get_col( $wpdb->prepare( "SELECT $type_column FROM $table WHERE meta_key = %s AND meta_value = %s", $meta_key, $meta_value ) );
369+
} else {
370+
$object_ids = $wpdb->get_col( $wpdb->prepare( "SELECT $type_column FROM $table WHERE meta_key = %s", $meta_key ) );
370371
}
371-
372-
$object_ids = $wpdb->get_col( $wpdb->prepare( "SELECT $type_column FROM $table WHERE meta_key = %s $value_clause", $meta_key ) );
373372
}
374373

375374
/**

Diff for: wp-includes/post.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -4317,10 +4317,10 @@ function get_page_by_path( $page_path, $output = OBJECT, $post_type = 'page' ) {
43174317
$page_path = str_replace('%2F', '/', $page_path);
43184318
$page_path = str_replace('%20', ' ', $page_path);
43194319
$parts = explode( '/', trim( $page_path, '/' ) );
4320-
$parts = esc_sql( $parts );
43214320
$parts = array_map( 'sanitize_title_for_query', $parts );
4321+
$escaped_parts = esc_sql( $parts );
43224322

4323-
$in_string = "'" . implode( "','", $parts ) . "'";
4323+
$in_string = "'" . implode( "','", $escaped_parts ) . "'";
43244324

43254325
if ( is_array( $post_type ) ) {
43264326
$post_types = $post_type;

Diff for: wp-includes/version.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*
55
* @global string $wp_version
66
*/
7-
$wp_version = '4.9-RC1-42050';
7+
$wp_version = '4.9-RC1-42056';
88

99
/**
1010
* Holds the WordPress DB revision, increments when changes are made to the WordPress DB schema.

Diff for: wp-includes/wp-db.php

+131-34
Original file line numberDiff line numberDiff line change
@@ -1106,20 +1106,22 @@ function _weak_escape( $string ) {
11061106
function _real_escape( $string ) {
11071107
if ( $this->dbh ) {
11081108
if ( $this->use_mysqli ) {
1109-
return mysqli_real_escape_string( $this->dbh, $string );
1109+
$escaped = mysqli_real_escape_string( $this->dbh, $string );
11101110
} else {
1111-
return mysql_real_escape_string( $string, $this->dbh );
1111+
$escaped = mysql_real_escape_string( $string, $this->dbh );
11121112
}
1113-
}
1114-
1115-
$class = get_class( $this );
1116-
if ( function_exists( '__' ) ) {
1117-
/* translators: %s: database access abstraction class, usually wpdb or a class extending wpdb */
1118-
_doing_it_wrong( $class, sprintf( __( '%s must set a database connection for use with escaping.' ), $class ), '3.6.0' );
11191113
} else {
1120-
_doing_it_wrong( $class, sprintf( '%s must set a database connection for use with escaping.', $class ), '3.6.0' );
1114+
$class = get_class( $this );
1115+
if ( function_exists( '__' ) ) {
1116+
/* translators: %s: database access abstraction class, usually wpdb or a class extending wpdb */
1117+
_doing_it_wrong( $class, sprintf( __( '%s must set a database connection for use with escaping.' ), $class ), '3.6.0' );
1118+
} else {
1119+
_doing_it_wrong( $class, sprintf( '%s must set a database connection for use with escaping.', $class ), '3.6.0' );
1120+
}
1121+
$escaped = addslashes( $string );
11211122
}
1122-
return addslashes( $string );
1123+
1124+
return $this->add_placeholder_escape( $escaped );
11231125
}
11241126

11251127
/**
@@ -1201,14 +1203,14 @@ public function escape_by_ref( &$string ) {
12011203
*
12021204
* All placeholders MUST be left unquoted in the query string. A corresponding argument MUST be passed for each placeholder.
12031205
*
1206+
* For compatibility with old behavior, numbered or formatted string placeholders (eg, %1$s, %5s) will not have quotes
1207+
* added by this function, so should be passed with appropriate quotes around them for your usage.
1208+
*
12041209
* Literal percentage signs (%) in the query string must be written as %%. Percentage wildcards (for example,
12051210
* to use in LIKE syntax) must be passed via a substitution argument containing the complete LIKE string, these
12061211
* cannot be inserted directly in the query string. Also see {@see esc_like()}.
12071212
*
1208-
* This method DOES NOT support sign, padding, alignment, width or precision specifiers.
1209-
* This method DOES NOT support argument numbering or swapping.
1210-
*
1211-
* Arguments may be passed as individual arguments to the method, or as a single array containing all arguments. A combination
1213+
* Arguments may be passed as individual arguments to the method, or as a single array containing all arguments. A combination
12121214
* of the two is not supported.
12131215
*
12141216
* Examples:
@@ -1225,8 +1227,9 @@ public function escape_by_ref( &$string ) {
12251227
* @return string|void Sanitized query string, if there is a query to prepare.
12261228
*/
12271229
public function prepare( $query, $args ) {
1228-
if ( is_null( $query ) )
1230+
if ( is_null( $query ) ) {
12291231
return;
1232+
}
12301233

12311234
// This is not meant to be foolproof -- but it will catch obviously incorrect usage.
12321235
if ( strpos( $query, '%' ) === false ) {
@@ -1237,8 +1240,10 @@ public function prepare( $query, $args ) {
12371240
$args = func_get_args();
12381241
array_shift( $args );
12391242

1240-
// If args were passed as an array (as in vsprintf), move them up
1243+
// If args were passed as an array (as in vsprintf), move them up.
1244+
$passed_as_array = false;
12411245
if ( is_array( $args[0] ) && count( $args ) == 1 ) {
1246+
$passed_as_array = true;
12421247
$args = $args[0];
12431248
}
12441249

@@ -1249,28 +1254,62 @@ public function prepare( $query, $args ) {
12491254
}
12501255
}
12511256

1252-
$query = str_replace( "'%s'", '%s', $query ); // in case someone mistakenly already singlequoted it
1253-
$query = str_replace( '"%s"', '%s', $query ); // doublequote unquoting
1254-
$query = preg_replace( '|(?<!%)%f|' , '%F', $query ); // Force floats to be locale unaware
1255-
$query = preg_replace( '|(?<!%)%s|', "'%s'", $query ); // quote the strings, avoiding escaped strings like %%s
1256-
$query = preg_replace( '/%(?:%|$|([^dsF]))/', '%%\\1', $query ); // escape any unescaped percents
1257+
/*
1258+
* Specify the formatting allowed in a placeholder. The following are allowed:
1259+
*
1260+
* - Sign specifier. eg, $+d
1261+
* - Numbered placeholders. eg, %1$s
1262+
* - Padding specifier, including custom padding characters. eg, %05s, %'#5s
1263+
* - Alignment specifier. eg, %05-s
1264+
* - Precision specifier. eg, %.2f
1265+
*/
1266+
$allowed_format = '(?:[1-9][0-9]*[$])?[-+0-9]*(?: |0|\'.)?[-+0-9]*(?:\.[0-9]+)?';
1267+
1268+
/*
1269+
* If a %s placeholder already has quotes around it, removing the existing quotes and re-inserting them
1270+
* ensures the quotes are consistent.
1271+
*
1272+
* For backwards compatibility, this is only applied to %s, and not to placeholders like %1$s, which are frequently
1273+
* used in the middle of longer strings, or as table name placeholders.
1274+
*/
1275+
$query = str_replace( "'%s'", '%s', $query ); // Strip any existing single quotes.
1276+
$query = str_replace( '"%s"', '%s', $query ); // Strip any existing double quotes.
1277+
$query = preg_replace( '/(?<!%)%s/', "'%s'", $query ); // Quote the strings, avoiding escaped strings like %%s.
1278+
1279+
$query = preg_replace( "/(?<!%)(%($allowed_format)?f)/" , '%\\2F', $query ); // Force floats to be locale unaware.
12571280

1258-
// Count the number of valid placeholders in the query
1259-
$placeholders = preg_match_all( '/(^|[^%]|(%%)+)%[sdF]/', $query, $matches );
1281+
$query = preg_replace( "/%(?:%|$|(?!($allowed_format)?[sdF]))/", '%%\\1', $query ); // Escape any unescaped percents.
12601282

1261-
if ( count ( $args ) !== $placeholders ) {
1262-
wp_load_translations_early();
1263-
_doing_it_wrong( 'wpdb::prepare',
1264-
/* translators: 1: number of placeholders, 2: number of arguments passed */
1265-
sprintf( __( 'The query does not contain the correct number of placeholders (%1$d) for the number of arguments passed (%2$d).' ),
1266-
$placeholders,
1267-
count( $args ) ),
1268-
'4.9.0'
1269-
);
1283+
// Count the number of valid placeholders in the query.
1284+
$placeholders = preg_match_all( "/(^|[^%]|(%%)+)%($allowed_format)?[sdF]/", $query, $matches );
1285+
1286+
if ( count( $args ) !== $placeholders ) {
1287+
if ( 1 === $placeholders && $passed_as_array ) {
1288+
// If the passed query only expected one argument, but the wrong number of arguments were sent as an array, bail.
1289+
wp_load_translations_early();
1290+
_doing_it_wrong( 'wpdb::prepare', __( 'The query only expected one placeholder, but an array of multiple placeholders was sent.' ), '4.9.0' );
1291+
1292+
return;
1293+
} else {
1294+
/*
1295+
* If we don't have the right number of placeholders, but they were passed as individual arguments,
1296+
* or we were expecting multiple arguments in an array, throw a warning.
1297+
*/
1298+
wp_load_translations_early();
1299+
_doing_it_wrong( 'wpdb::prepare',
1300+
/* translators: 1: number of placeholders, 2: number of arguments passed */
1301+
sprintf( __( 'The query does not contain the correct number of placeholders (%1$d) for the number of arguments passed (%2$d).' ),
1302+
$placeholders,
1303+
count( $args ) ),
1304+
'4.8.3'
1305+
);
1306+
}
12701307
}
12711308

12721309
array_walk( $args, array( $this, 'escape_by_ref' ) );
1273-
return @vsprintf( $query, $args );
1310+
$query = @vsprintf( $query, $args );
1311+
1312+
return $this->add_placeholder_escape( $query );
12741313
}
12751314

12761315
/**
@@ -1893,6 +1932,64 @@ private function _do_query( $query ) {
18931932
}
18941933
}
18951934

1935+
/**
1936+
* Generates and returns a placeholder escape string for use in queries returned by ::prepare().
1937+
*
1938+
* @since 4.8.3
1939+
*
1940+
* @return string String to escape placeholders.
1941+
*/
1942+
public function placeholder_escape() {
1943+
static $placeholder;
1944+
1945+
if ( ! $placeholder ) {
1946+
// If ext/hash is not present, compat.php's hash_hmac() does not support sha256.
1947+
$algo = function_exists( 'hash' ) ? 'sha256' : 'sha1';
1948+
// Old WP installs may not have AUTH_SALT defined.
1949+
$salt = defined( 'AUTH_SALT' ) ? AUTH_SALT : rand();
1950+
1951+
$placeholder = '{' . hash_hmac( $algo, uniqid( $salt, true ), $salt ) . '}';
1952+
}
1953+
1954+
/*
1955+
* Add the filter to remove the placeholder escaper. Uses priority 0, so that anything
1956+
* else attached to this filter will recieve the query with the placeholder string removed.
1957+
*/
1958+
if ( ! has_filter( 'query', array( $this, 'remove_placeholder_escape' ) ) ) {
1959+
add_filter( 'query', array( $this, 'remove_placeholder_escape' ), 0 );
1960+
}
1961+
1962+
return $placeholder;
1963+
}
1964+
1965+
/**
1966+
* Adds a placeholder escape string, to escape anything that resembles a printf() placeholder.
1967+
*
1968+
* @since 4.8.3
1969+
*
1970+
* @param string $query The query to escape.
1971+
* @return string The query with the placeholder escape string inserted where necessary.
1972+
*/
1973+
public function add_placeholder_escape( $query ) {
1974+
/*
1975+
* To prevent returning anything that even vaguely resembles a placeholder,
1976+
* we clobber every % we can find.
1977+
*/
1978+
return str_replace( '%', $this->placeholder_escape(), $query );
1979+
}
1980+
1981+
/**
1982+
* Removes the placeholder escape strings from a query.
1983+
*
1984+
* @since 4.8.3
1985+
*
1986+
* @param string $query The query from which the placeholder will be removed.
1987+
* @return string The query with the placeholder removed.
1988+
*/
1989+
public function remove_placeholder_escape( $query ) {
1990+
return str_replace( $this->placeholder_escape(), '%', $query );
1991+
}
1992+
18961993
/**
18971994
* Insert a row into a table.
18981995
*
@@ -2064,7 +2161,7 @@ public function update( $table, $data, $where, $format = null, $where_format = n
20642161
$conditions = implode( ' AND ', $conditions );
20652162

20662163
$sql = "UPDATE `$table` SET $fields WHERE $conditions";
2067-
2164+
20682165
$this->check_current_query = false;
20692166
return $this->query( $this->prepare( $sql, $values ) );
20702167
}

0 commit comments

Comments
 (0)