From a59f4bc10f2b651099cdacd018ef85b31f01a48f Mon Sep 17 00:00:00 2001 From: Gary Pendergast Date: Tue, 31 Oct 2017 12:23:33 +0000 Subject: [PATCH] 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. Merges [41662], [42056] to the 4.8 branch. See #41925. Built from https://develop.svn.wordpress.org/branches/4.8@42057 git-svn-id: http://core.svn.wordpress.org/branches/4.8@41886 1a063a9b-81f0-0310-95a4-ce76da25c4cd --- wp-includes/formatting.php | 5 + wp-includes/post.php | 4 +- wp-includes/wp-db.php | 181 ++++++++++++++++++++++++++++++------- 3 files changed, 154 insertions(+), 36 deletions(-) diff --git a/wp-includes/formatting.php b/wp-includes/formatting.php index 9e5f8b4147..2a88509fca 100644 --- a/wp-includes/formatting.php +++ b/wp-includes/formatting.php @@ -3738,6 +3738,11 @@ function _deep_replace( $search, $subject ) { * Sometimes, spot-escaping is required or useful. One example * is preparing an array for use in an IN clause. * + * NOTE: Since 4.8.3, '%' characters will be replaced with a placeholder string, + * this prevents certain SQLi attacks from taking place. This change in behaviour + * may cause issues for code that expects the return value of esc_sql() to be useable + * for other purposes. + * * @since 2.8.0 * * @global wpdb $wpdb WordPress database abstraction object. diff --git a/wp-includes/post.php b/wp-includes/post.php index 0d624006d7..c39939629f 100644 --- a/wp-includes/post.php +++ b/wp-includes/post.php @@ -4246,10 +4246,10 @@ function get_page_by_path( $page_path, $output = OBJECT, $post_type = 'page' ) { $page_path = str_replace('%2F', '/', $page_path); $page_path = str_replace('%20', ' ', $page_path); $parts = explode( '/', trim( $page_path, '/' ) ); - $parts = esc_sql( $parts ); $parts = array_map( 'sanitize_title_for_query', $parts ); + $escaped_parts = esc_sql( $parts ); - $in_string = "'" . implode( "','", $parts ) . "'"; + $in_string = "'" . implode( "','", $escaped_parts ) . "'"; if ( is_array( $post_type ) ) { $post_types = $post_type; diff --git a/wp-includes/wp-db.php b/wp-includes/wp-db.php index ced76bd867..553cd39d9f 100644 --- a/wp-includes/wp-db.php +++ b/wp-includes/wp-db.php @@ -1168,20 +1168,22 @@ class wpdb { function _real_escape( $string ) { if ( $this->dbh ) { if ( $this->use_mysqli ) { - return mysqli_real_escape_string( $this->dbh, $string ); + $escaped = mysqli_real_escape_string( $this->dbh, $string ); } else { - return mysql_real_escape_string( $string, $this->dbh ); + $escaped = mysql_real_escape_string( $string, $this->dbh ); } + } else { + $class = get_class( $this ); + if ( function_exists( '__' ) ) { + /* translators: %s: database access abstraction class, usually wpdb or a class extending wpdb */ + _doing_it_wrong( $class, sprintf( __( '%s must set a database connection for use with escaping.' ), $class ), '3.6.0' ); + } else { + _doing_it_wrong( $class, sprintf( '%s must set a database connection for use with escaping.', $class ), '3.6.0' ); + } + $escaped = addslashes( $string ); } - $class = get_class( $this ); - if ( function_exists( '__' ) ) { - /* translators: %s: database access abstraction class, usually wpdb or a class extending wpdb */ - _doing_it_wrong( $class, sprintf( __( '%s must set a database connection for use with escaping.' ), $class ), '3.6.0' ); - } else { - _doing_it_wrong( $class, sprintf( '%s must set a database connection for use with escaping.', $class ), '3.6.0' ); - } - return addslashes( $string ); + return $this->add_placeholder_escape( $escaped ); } /** @@ -1257,67 +1259,120 @@ class wpdb { /** * Prepares a SQL query for safe execution. Uses sprintf()-like syntax. * - * The following directives can be used in the query format string: + * The following placeholders can be used in the query string: * %d (integer) * %f (float) * %s (string) - * %% (literal percentage sign - no argument needed) * - * All of %d, %f, and %s are to be left unquoted in the query string and they need an argument passed for them. - * Literals (%) as parts of the query must be properly written as %%. + * All placeholders MUST be left unquoted in the query string. A corresponding argument MUST be passed for each placeholder. * - * This function only supports a small subset of the sprintf syntax; it only supports %d (integer), %f (float), and %s (string). - * Does not support sign, padding, alignment, width or precision specifiers. - * Does not support argument numbering/swapping. + * For compatibility with old behavior, numbered or formatted string placeholders (eg, %1$s, %5s) will not have quotes + * added by this function, so should be passed with appropriate quotes around them for your usage. * - * May be called like {@link https://secure.php.net/sprintf sprintf()} or like {@link https://secure.php.net/vsprintf vsprintf()}. + * Literal percentage signs (%) in the query string must be written as %%. Percentage wildcards (for example, + * to use in LIKE syntax) must be passed via a substitution argument containing the complete LIKE string, these + * cannot be inserted directly in the query string. Also see {@see esc_like()}. * - * Both %d and %s should be left unquoted in the query string. + * Arguments may be passed as individual arguments to the method, or as a single array containing all arguments. A combination + * of the two is not supported. * - * $wpdb->prepare( "SELECT * FROM `table` WHERE `column` = %s AND `field` = %d", 'foo', 1337 ); + * Examples: + * $wpdb->prepare( "SELECT * FROM `table` WHERE `column` = %s AND `field` = %d OR `other_field` LIKE %s", array( 'foo', 1337, '%bar' ) ); * $wpdb->prepare( "SELECT DATE_FORMAT(`field`, '%%c') FROM `table` WHERE `column` = %s", 'foo' ); * * @link https://secure.php.net/sprintf Description of syntax. * @since 2.3.0 * * @param string $query Query statement with sprintf()-like placeholders - * @param array|mixed $args The array of variables to substitute into the query's placeholders if being called like - * {@link https://secure.php.net/vsprintf vsprintf()}, or the first variable to substitute into the query's placeholders if - * being called like {@link https://secure.php.net/sprintf sprintf()}. - * @param mixed $args,... further variables to substitute into the query's placeholders if being called like - * {@link https://secure.php.net/sprintf sprintf()}. + * @param array|mixed $args The array of variables to substitute into the query's placeholders if being called with an array of arguments, + * or the first variable to substitute into the query's placeholders if being called with individual arguments. + * @param mixed $args,... further variables to substitute into the query's placeholders if being called wih individual arguments. * @return string|void Sanitized query string, if there is a query to prepare. */ public function prepare( $query, $args ) { - if ( is_null( $query ) ) + if ( is_null( $query ) ) { return; + } // This is not meant to be foolproof -- but it will catch obviously incorrect usage. if ( strpos( $query, '%' ) === false ) { + wp_load_translations_early(); _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 args were passed as an array (as in vsprintf), move them up. + $passed_as_array = false; if ( is_array( $args[0] ) && count( $args ) == 1 ) { + $passed_as_array = true; $args = $args[0]; } foreach ( $args as $arg ) { if ( ! is_scalar( $arg ) && ! is_null( $arg ) ) { - _doing_it_wrong( 'wpdb::prepare', sprintf( 'Unsupported value type (%s).', gettype( $arg ) ), '4.8.2' ); + wp_load_translations_early(); + _doing_it_wrong( 'wpdb::prepare', sprintf( __( 'Unsupported value type (%s).' ), gettype( $arg ) ), '4.8.2' ); + } + } + + /* + * Specify the formatting allowed in a placeholder. The following are allowed: + * + * - Sign specifier. eg, $+d + * - Numbered placeholders. eg, %1$s + * - Padding specifier, including custom padding characters. eg, %05s, %'#5s + * - Alignment specifier. eg, %05-s + * - Precision specifier. eg, %.2f + */ + $allowed_format = '(?:[1-9][0-9]*[$])?[-+0-9]*(?: |0|\'.)?[-+0-9]*(?:\.[0-9]+)?'; + + /* + * If a %s placeholder already has quotes around it, removing the existing quotes and re-inserting them + * ensures the quotes are consistent. + * + * For backwards compatibility, this is only applied to %s, and not to placeholders like %1$s, which are frequently + * used in the middle of longer strings, or as table name placeholders. + */ + $query = str_replace( "'%s'", '%s', $query ); // Strip any existing single quotes. + $query = str_replace( '"%s"', '%s', $query ); // Strip any existing double quotes. + $query = preg_replace( '/(?add_placeholder_escape( $query ); } /** @@ -1895,6 +1950,64 @@ class wpdb { } } + /** + * Generates and returns a placeholder escape string for use in queries returned by ::prepare(). + * + * @since 4.8.3 + * + * @return string String to escape placeholders. + */ + public function placeholder_escape() { + static $placeholder; + + if ( ! $placeholder ) { + // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + // Old WP installs may not have AUTH_SALT defined. + $salt = defined( 'AUTH_SALT' ) ? AUTH_SALT : rand(); + + $placeholder = '{' . hash_hmac( $algo, uniqid( $salt, true ), $salt ) . '}'; + } + + /* + * Add the filter to remove the placeholder escaper. Uses priority 0, so that anything + * else attached to this filter will recieve the query with the placeholder string removed. + */ + if ( ! has_filter( 'query', array( $this, 'remove_placeholder_escape' ) ) ) { + add_filter( 'query', array( $this, 'remove_placeholder_escape' ), 0 ); + } + + return $placeholder; + } + + /** + * Adds a placeholder escape string, to escape anything that resembles a printf() placeholder. + * + * @since 4.8.3 + * + * @param string $query The query to escape. + * @return string The query with the placeholder escape string inserted where necessary. + */ + public function add_placeholder_escape( $query ) { + /* + * To prevent returning anything that even vaguely resembles a placeholder, + * we clobber every % we can find. + */ + return str_replace( '%', $this->placeholder_escape(), $query ); + } + + /** + * Removes the placeholder escape strings from a query. + * + * @since 4.8.3 + * + * @param string $query The query from which the placeholder will be removed. + * @return string The query with the placeholder removed. + */ + public function remove_placeholder_escape( $query ) { + return str_replace( $this->placeholder_escape(), '%', $query ); + } + /** * Insert a row into a table. *