WordPress.org
Get WordPress

Make WordPress Core

Changeset 54734


Ignore:
Timestamp:
10/31/2022 08:43:56 PM (2 years ago)
Author:
SergeyBiryukov
Message:

Database: Revert [53575].

When using '%%%s%%' pattern with $wpdb->prepare(), it works on 6.0.3 but does not on 6.1-RC. Why? The inserted value is wrapped in quotes on 6.1-RC5 whereas it is not on <= 6.0.3.

With 6.1 final release tomorrow, more time is needed to further investigate and test. Reverting this changeset to restore the previous behavior.

This commit also adds a dataset for testing the '%%%s%%' pattern.

Props SergeyBiryukov, hellofromTonya, bernhard-reiter, desrosj, davidbaumwald, jorbin.
Reviewed by hellofromTonya, SergeyBiryukov.
Merges [54733] to the 6.1 branch.
Fixes #56933.
See #52506.

Location:
branches/6.1
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • branches/6.1

  • branches/6.1/src/wp-includes/class-wpdb.php

    r54384 r54734  
    656656
    657657    /**
    658      * Backward compatibility, where wpdb::prepare() has not quoted formatted/argnum placeholders.
    659      *
    660      * Historically this could be used for table/field names, or for some string formatting, e.g.
    661      *
    662      *     $wpdb->prepare( 'WHERE `%1s` = "%1s something %1s" OR %1$s = "%-10s"', 'field_1', 'a', 'b', 'c' );
    663      *
    664      * But it's risky, e.g. forgetting to add quotes, resulting in SQL Injection vulnerabilities:
    665      *
    666      *     $wpdb->prepare( 'WHERE (id = %1s) OR (id = %2$s)', $_GET['id'], $_GET['id'] ); // ?id=id
    667      *
    668      * This feature is preserved while plugin authors update their code to use safer approaches:
    669      *
    670      *     $wpdb->prepare( 'WHERE %1s = %s', $_GET['key'], $_GET['value'] );
    671      *     $wpdb->prepare( 'WHERE %i  = %s', $_GET['key'], $_GET['value'] );
    672      *
    673      * While changing to false will be fine for queries not using formatted/argnum placeholders,
    674      * any remaining cases are most likely going to result in SQL errors (good, in a way):
    675      *
    676      *     $wpdb->prepare( 'WHERE %1s = "%-10s"', 'my_field', 'my_value' );
    677      *     true  = WHERE my_field = "my_value  "
    678      *     false = WHERE 'my_field' = "'my_value  '"
    679      *
    680      * But there may be some queries that result in an SQL Injection vulnerability:
    681      *
    682      *     $wpdb->prepare( 'WHERE id = %1s', $_GET['id'] ); // ?id=id
    683      *
    684      * So there may need to be a `_doing_it_wrong()` phase, after we know everyone can use
    685      * identifier placeholders (%i), but before this feature is disabled or removed.
    686      *
    687      * @since 6.1.0
    688      * @var bool
    689      */
    690     private $allow_unsafe_unquoted_parameters = true;
    691 
    692     /**
    693658     * Whether to use mysqli over mysql. Default false.
    694659     *
    … …  
    13991364
    14001365    /**
    1401      * Escapes an identifier for a MySQL database, e.g. table/field names.
    1402      *
    1403      * @since 6.1.0
    1404      *
    1405      * @param string $identifier Identifier to escape.
    1406      * @return string Escaped identifier.
    1407      */
    1408     public function escape_identifier( $identifier ) {
    1409         return '`' . $this->_escape_identifier_value( $identifier ) . '`';
    1410     }
    1411 
    1412     /**
    1413      * Escapes an identifier value without adding the surrounding quotes.
    1414      *
    1415      * - Permitted characters in quoted identifiers include the full Unicode
    1416      *   Basic Multilingual Plane (BMP), except U+0000.
    1417      * - To quote the identifier itself, you need to double the character, e.g. `a``b`.
    1418      *
    1419      * @since 6.1.0
    1420      * @access private
    1421      *
    1422      * @link https://dev.mysql.com/doc/refman/8.0/en/identifiers.html
    1423      *
    1424      * @param string $identifier Identifier to escape.
    1425      * @return string Escaped identifier.
    1426      */
    1427     private function _escape_identifier_value( $identifier ) {
    1428         return str_replace( '`', '``', $identifier );
    1429     }
    1430 
    1431     /**
    14321366     * Prepares a SQL query for safe execution.
    14331367     *
    … …  
    14371371     * - %f (float)
    14381372     * - %s (string)
    1439      * - %i (identifier, e.g. table/field names)
    14401373     *
    14411374     * All placeholders MUST be left unquoted in the query string. A corresponding argument
    … …  
    14701403     *              by updating the function signature. The second parameter was changed
    14711404     *              from `$args` to `...$args`.
    1472      * @since 6.1.0 Added `%i` for identifiers, e.g. table or field names.
    1473      *              Check support via `wpdb::has_cap( 'identifier_placeholders' )`.
    1474      *              This preserves compatibility with sprintf(), as the C version uses
    1475      *              `%d` and `$i` as a signed integer, whereas PHP only supports `%d`.
    14761405     *
    14771406     * @link https://www.php.net/sprintf Description of syntax.
    … …  
    15051434        }
    15061435
     1436        // If args were passed as an array (as in vsprintf), move them up.
     1437        $passed_as_array = false;
     1438        if ( isset( $args[0] ) && is_array( $args[0] ) && 1 === count( $args ) ) {
     1439            $passed_as_array = true;
     1440            $args            = $args[0];
     1441        }
     1442
     1443        foreach ( $args as $arg ) {
     1444            if ( ! is_scalar( $arg ) && ! is_null( $arg ) ) {
     1445                wp_load_translations_early();
     1446                _doing_it_wrong(
     1447                    'wpdb::prepare',
     1448                    sprintf(
     1449                        /* translators: %s: Value type. */
     1450                        __( 'Unsupported value type (%s).' ),
     1451                        gettype( $arg )
     1452                    ),
     1453                    '4.8.2'
     1454                );
     1455            }
     1456        }
     1457
    15071458        /*
    15081459         * Specify the formatting allowed in a placeholder. The following are allowed:
    … …  
    15251476        $query = str_replace( "'%s'", '%s', $query ); // Strip any existing single quotes.
    15261477        $query = str_replace( '"%s"', '%s', $query ); // Strip any existing double quotes.
    1527 
    1528         // Escape any unescaped percents (i.e. anything unrecognised).
    1529         $query = preg_replace( "/%(?:%|$|(?!($allowed_format)?[sdfFi]))/", '%%\\1', $query );
    1530 
    1531         // Extract placeholders from the query.
    1532         $split_query = preg_split( "/(^|[^%]|(?:%%)+)(%(?:$allowed_format)?[sdfFi])/", $query, -1, PREG_SPLIT_DELIM_CAPTURE );
    1533 
    1534         $split_query_count = count( $split_query );
    1535         /*
    1536          * Split always returns with 1 value before the first placeholder (even with $query = "%s"),
    1537          * then 3 additional values per placeholder.
    1538          */
    1539         $placeholder_count = ( ( $split_query_count - 1 ) / 3 );
    1540 
    1541         // If args were passed as an array, as in vsprintf(), move them up.
    1542         $passed_as_array = ( isset( $args[0] ) && is_array( $args[0] ) && 1 === count( $args ) );
    1543         if ( $passed_as_array ) {
    1544             $args = $args[0];
    1545         }
    1546 
    1547         $new_query       = '';
    1548         $key             = 2; // Keys 0 and 1 in $split_query contain values before the first placeholder.
    1549         $arg_id          = 0;
    1550         $arg_identifiers = array();
    1551         $arg_strings     = array();
    1552 
    1553         while ( $key < $split_query_count ) {
    1554             $placeholder = $split_query[ $key ];
    1555 
    1556             $format = substr( $placeholder, 1, -1 );
    1557             $type   = substr( $placeholder, -1 );
    1558 
    1559             if ( 'f' === $type ) { // Force floats to be locale-unaware.
    1560                 $type        = 'F';
    1561                 $placeholder = '%' . $format . $type;
    1562             }
    1563 
    1564             if ( 'i' === $type ) {
    1565                 $placeholder = '`%' . $format . 's`';
    1566                 // Using a simple strpos() due to previous checking (e.g. $allowed_format).
    1567                 $argnum_pos = strpos( $format, '$' );
    1568 
    1569                 if ( false !== $argnum_pos ) {
    1570                     // sprintf() argnum starts at 1, $arg_id from 0.
    1571                     $arg_identifiers[] = ( intval( substr( $format, 0, $argnum_pos ) ) - 1 );
    1572                 } else {
    1573                     $arg_identifiers[] = $arg_id;
    1574                 }
    1575             } elseif ( 'd' !== $type && 'F' !== $type ) {
    1576                 /*
    1577                  * i.e. ( 's' === $type ), where 'd' and 'F' keeps $placeholder unchanged,
    1578                  * and we ensure string escaping is used as a safe default (e.g. even if 'x').
    1579                  */
    1580                 $argnum_pos = strpos( $format, '$' );
    1581 
    1582                 if ( false !== $argnum_pos ) {
    1583                     $arg_strings[] = ( intval( substr( $format, 0, $argnum_pos ) ) - 1 );
    1584                 }
    1585 
    1586                 // Unquoted strings for backward compatibility (dangerous).
    1587                 if ( true !== $this->allow_unsafe_unquoted_parameters || '' === $format ) {
    1588                     $placeholder = "'%" . $format . "s'";
    1589                 }
    1590             }
    1591 
    1592             // Glue (-2), any leading characters (-1), then the new $placeholder.
    1593             $new_query .= $split_query[ $key - 2 ] . $split_query[ $key - 1 ] . $placeholder;
    1594 
    1595             $key += 3;
    1596             $arg_id++;
    1597         }
    1598 
    1599         // Replace $query; and add remaining $query characters, or index 0 if there were no placeholders.
    1600         $query = $new_query . $split_query[ $key - 2 ];
    1601 
    1602         $dual_use = array_intersect( $arg_identifiers, $arg_strings );
    1603 
    1604         if ( count( $dual_use ) ) {
    1605             wp_load_translations_early();
    1606             _doing_it_wrong(
    1607                 'wpdb::prepare',
    1608                 sprintf(
    1609                     /* translators: %s: A comma-separated list of arguments found to be a problem. */
    1610                     __( 'Arguments (%s) cannot be used for both String and Identifier escaping.' ),
    1611                     implode( ', ', $dual_use )
    1612                 ),
    1613                 '6.1.0'
    1614             );
    1615 
    1616             return;
    1617         }
     1478        $query = preg_replace( '/(?<!%)%s/', "'%s'", $query ); // Quote the strings, avoiding escaped strings like %%s.
     1479
     1480        $query = preg_replace( "/(?<!%)(%($allowed_format)?f)/", '%\\2F', $query ); // Force floats to be locale-unaware.
     1481
     1482        $query = preg_replace( "/%(?:%|$|(?!($allowed_format)?[sdF]))/", '%%\\1', $query ); // Escape any unescaped percents.
     1483
     1484        // Count the number of valid placeholders in the query.
     1485        $placeholders = preg_match_all( "/(^|[^%]|(%%)+)%($allowed_format)?[sdF]/", $query, $matches );
    16181486
    16191487        $args_count = count( $args );
    16201488
    1621         if ( $args_count !== $placeholder_count ) {
    1622             if ( 1 === $placeholder_count && $passed_as_array ) {
    1623                 /*
    1624                  * If the passed query only expected one argument,
    1625                  * but the wrong number of arguments was sent as an array, bail.
    1626                  */
     1489        if ( $args_count !== $placeholders ) {
     1490            if ( 1 === $placeholders && $passed_as_array ) {
     1491                // If the passed query only expected one argument, but the wrong number of arguments were sent as an array, bail.
    16271492                wp_load_translations_early();
    16281493                _doing_it_wrong(
    … …  
    16451510                        /* translators: 1: Number of placeholders, 2: Number of arguments passed. */
    16461511                        __( 'The query does not contain the correct number of placeholders (%1$d) for the number of arguments passed (%2$d).' ),
    1647                         $placeholder_count,
     1512                        $placeholders,
    16481513                        $args_count
    16491514                    ),
    … …  
    16551520                 * return an empty string to avoid a fatal error on PHP 8.
    16561521                 */
    1657                 if ( $args_count < $placeholder_count ) {
    1658                     $max_numbered_placeholder = 0;
    1659 
    1660                     for ( $i = 2, $l = $split_query_count; $i < $l; $i += 3 ) {
    1661                         // Assume a leading number is for a numbered placeholder, e.g. '%3$s'.
    1662                         $argnum = intval( substr( $split_query[ $i ], 1 ) );
    1663 
    1664                         if ( $max_numbered_placeholder < $argnum ) {
    1665                             $max_numbered_placeholder = $argnum;
    1666                         }
    1667                     }
     1522                if ( $args_count < $placeholders ) {
     1523                    $max_numbered_placeholder = ! empty( $matches[3] ) ? max( array_map( 'intval', $matches[3] ) ) : 0;
    16681524
    16691525                    if ( ! $max_numbered_placeholder || $args_count < $max_numbered_placeholder ) {
    … …  
    16741530        }
    16751531
    1676         $args_escaped = array();
    1677 
    1678         foreach ( $args as $i => $value ) {
    1679             if ( in_array( $i, $arg_identifiers, true ) ) {
    1680                 $args_escaped[] = $this->_escape_identifier_value( $value );
    1681             } elseif ( is_int( $value ) || is_float( $value ) ) {
    1682                 $args_escaped[] = $value;
    1683             } else {
    1684                 if ( ! is_scalar( $value ) && ! is_null( $value ) ) {
    1685                     wp_load_translations_early();
    1686                     _doing_it_wrong(
    1687                         'wpdb::prepare',
    1688                         sprintf(
    1689                             /* translators: %s: Value type. */
    1690                             __( 'Unsupported value type (%s).' ),
    1691                             gettype( $value )
    1692                         ),
    1693                         '4.8.2'
    1694                     );
    1695 
    1696                     // Preserving old behavior, where values are escaped as strings.
    1697                     $value = '';
    1698                 }
    1699 
    1700                 $args_escaped[] = $this->_real_escape( $value );
    1701             }
    1702         }
    1703 
    1704         $query = vsprintf( $query, $args_escaped );
     1532        array_walk( $args, array( $this, 'escape_by_ref' ) );
     1533        $query = vsprintf( $query, $args );
    17051534
    17061535        return $this->add_placeholder_escape( $query );
    … …  
    39513780     * @since 4.1.0 Added support for the 'utf8mb4' feature.
    39523781     * @since 4.6.0 Added support for the 'utf8mb4_520' feature.
    3953      * @since 6.1.0 Added support for the 'identifier_placeholders' feature.
    39543782     *
    39553783     * @see wpdb::db_version()
    39563784     *
    39573785     * @param string $db_cap The feature to check for. Accepts 'collation', 'group_concat',
    3958      *                       'subqueries', 'set_charset', 'utf8mb4', 'utf8mb4_520',
    3959      *                       or 'identifier_placeholders'.
     3786     *                       'subqueries', 'set_charset', 'utf8mb4', or 'utf8mb4_520'.
    39603787     * @return bool True when the database feature is supported, false otherwise.
    39613788     */
    … …  
    40023829            case 'utf8mb4_520': // @since 4.6.0
    40033830                return version_compare( $db_version, '5.6', '>=' );
    4004             case 'identifier_placeholders': // @since 6.1.0
    4005                 /*
    4006                  * As of WordPress 6.1, wpdb::prepare() supports identifiers via '%i',
    4007                  * e.g. table/field names.
    4008                  */
    4009                 return true;
    40103831        }
    40113832

  • branches/6.1/tests/phpunit/tests/db.php

    r53670 r54734  
    495495        $this->assertTrue( $wpdb->has_cap( 'group_concat' ) );
    496496        $this->assertTrue( $wpdb->has_cap( 'subqueries' ) );
    497         $this->assertTrue( $wpdb->has_cap( 'identifier_placeholders' ) );
    498497        $this->assertTrue( $wpdb->has_cap( 'COLLATION' ) );
    499498        $this->assertTrue( $wpdb->has_cap( 'GROUP_CONCAT' ) );
    500499        $this->assertTrue( $wpdb->has_cap( 'SUBQUERIES' ) );
    501         $this->assertTrue( $wpdb->has_cap( 'IDENTIFIER_PLACEHOLDERS' ) );
    502500        $this->assertSame(
    503501            version_compare( $wpdb->db_version(), '5.0.7', '>=' ),
    … …  
    17181716                "'{$placeholder_escape}'{$placeholder_escape}s 'hello'",
    17191717            ),
     1718            /*
     1719             * @ticket 56933.
     1720             * When preparing a '%%%s%%', test that the inserted value
     1721             * is not wrapped in single quotes between the 2 hex values.
     1722             */
     1723            array(
     1724                '%%%s%%',
     1725                'hello',
     1726                false,
     1727                "{$placeholder_escape}hello{$placeholder_escape}",
     1728            ),
    17201729            array(
    17211730                "'%-'#5s' '%'#-+-5s'",
    … …  
    17241733                "'hello' 'foo##'",
    17251734            ),
    1726             array(
    1727                 'SELECT * FROM %i WHERE %i = %d;',
    1728                 array( 'my_table', 'my_field', 321 ),
    1729                 false,
    1730                 'SELECT * FROM `my_table` WHERE `my_field` = 321;',
    1731             ),
    1732             array(
    1733                 'WHERE %i = %d;',
    1734                 array( 'evil_`_field', 321 ),
    1735                 false,
    1736                 'WHERE `evil_``_field` = 321;', // To quote the identifier itself, then you need to double the character, e.g. `a``b`.
    1737             ),
    1738             array(
    1739                 'WHERE %i = %d;',
    1740                 array( 'evil_````````_field', 321 ),
    1741                 false,
    1742                 'WHERE `evil_````````````````_field` = 321;',
    1743             ),
    1744             array(
    1745                 'WHERE %i = %d;',
    1746                 array( '``evil_field``', 321 ),
    1747                 false,
    1748                 'WHERE `````evil_field````` = 321;',
    1749             ),
    1750             array(
    1751                 'WHERE %i = %d;',
    1752                 array( 'evil\'field', 321 ),
    1753                 false,
    1754                 'WHERE `evil\'field` = 321;',
    1755             ),
    1756             array(
    1757                 'WHERE %i = %d;',
    1758                 array( 'evil_\``_field', 321 ),
    1759                 false,
    1760                 'WHERE `evil_\````_field` = 321;',
    1761             ),
    1762             array(
    1763                 'WHERE %i = %d;',
    1764                 array( 'evil_%s_field', 321 ),
    1765                 false,
    1766                 "WHERE `evil_{$placeholder_escape}s_field` = 321;",
    1767             ),
    1768             array(
    1769                 'WHERE %i = %d;',
    1770                 array( 'value`', 321 ),
    1771                 false,
    1772                 'WHERE `value``` = 321;',
    1773             ),
    1774             array(
    1775                 'WHERE `%i = %d;',
    1776                 array( ' AND evil_value', 321 ),
    1777                 false,
    1778                 'WHERE `` AND evil_value` = 321;', // Won't run (SQL parse error: "Unclosed quote").
    1779             ),
    1780             array(
    1781                 'WHERE %i` = %d;',
    1782                 array( 'evil_value -- ', 321 ),
    1783                 false,
    1784                 'WHERE `evil_value -- `` = 321;', // Won't run (SQL parse error: "Unclosed quote").
    1785             ),
    1786             array(
    1787                 'WHERE `%i`` = %d;',
    1788                 array( ' AND true -- ', 321 ),
    1789                 false,
    1790                 'WHERE `` AND true -- ``` = 321;', // Won't run (Unknown column '').
    1791             ),
    1792             array(
    1793                 'WHERE ``%i` = %d;',
    1794                 array( ' AND true -- ', 321 ),
    1795                 false,
    1796                 'WHERE ``` AND true -- `` = 321;', // Won't run (SQL parse error: "Unclosed quote").
    1797             ),
    1798             array(
    1799                 'WHERE %2$i = %1$d;',
    1800                 array( '1', 'two' ),
    1801                 false,
    1802                 'WHERE `two` = 1;',
    1803             ),
    1804             array(
    1805                 'WHERE \'%i\' = 1 AND "%i" = 2 AND `%i` = 3 AND ``%i`` = 4 AND %15i = 5',
    1806                 array( 'my_field1', 'my_field2', 'my_field3', 'my_field4', 'my_field5' ),
    1807                 false,
    1808                 'WHERE \'`my_field1`\' = 1 AND "`my_field2`" = 2 AND ``my_field3`` = 3 AND ```my_field4``` = 4 AND `      my_field5` = 5', // Does not remove any existing quotes, always adds it's own (safer).
    1809             ),
    1810             array(
    1811                 'WHERE id = %d AND %i LIKE %2$s LIMIT 1',
    1812                 array( 123, 'field -- ', false ),
    1813                 true, // Incorrect usage.
    1814                 null, // Should be rejected, otherwise the `%1$s` could use Identifier escaping, e.g. 'WHERE `field -- ` LIKE field --  LIMIT 1' (thanks @vortfu).
    1815             ),
    1816             array(
    1817                 'WHERE %i LIKE %s LIMIT 1',
    1818                 array( "field' -- ", "field' -- " ),
    1819                 false,
    1820                 "WHERE `field' -- ` LIKE 'field\' -- ' LIMIT 1", // In contrast to the above, Identifier vs String escaping is used.
    1821             ),
    1822         );
    1823     }
    1824 
    1825     public function test_allow_unsafe_unquoted_parameters() {
    1826         global $wpdb;
    1827 
    1828         $sql    = 'WHERE (%i = %s) OR (%10i = %10s) OR (%5$i = %6$s)';
    1829         $values = array( 'field_a', 'string_a', 'field_b', 'string_b', 'field_c', 'string_c' );
    1830 
    1831         $default = $wpdb->allow_unsafe_unquoted_parameters;
    1832 
    1833         $wpdb->allow_unsafe_unquoted_parameters = true;
    1834 
    1835         // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
    1836         $part = $wpdb->prepare( $sql, $values );
    1837         $this->assertSame( 'WHERE (`field_a` = \'string_a\') OR (`   field_b` =   string_b) OR (`field_c` = string_c)', $part ); // Unsafe, unquoted parameters.
    1838 
    1839         $wpdb->allow_unsafe_unquoted_parameters = false;
    1840 
    1841         // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
    1842         $part = $wpdb->prepare( $sql, $values );
    1843         $this->assertSame( 'WHERE (`field_a` = \'string_a\') OR (`   field_b` = \'  string_b\') OR (`field_c` = \'string_c\')', $part );
    1844 
    1845         $wpdb->allow_unsafe_unquoted_parameters = $default;
    1846 
     1735        );
    18471736    }
    18481737
Note: See TracChangeset for help on using the changeset viewer.