Ticket #56933: 56933.diff
File 56933.diff, 17.4 KB (added by , 2 years ago) |
---|
-
src/wp-includes/class-wpdb.php
655 655 ); 656 656 657 657 /** 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=id667 *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=id683 *684 * So there may need to be a `_doing_it_wrong()` phase, after we know everyone can use685 * identifier placeholders (%i), but before this feature is disabled or removed.686 *687 * @since 6.1.0688 * @var bool689 */690 private $allow_unsafe_unquoted_parameters = true;691 692 /**693 658 * Whether to use mysqli over mysql. Default false. 694 659 * 695 660 * @since 3.9.0 … … 1398 1363 } 1399 1364 1400 1365 /** 1401 * Escapes an identifier for a MySQL database, e.g. table/field names.1402 *1403 * @since 6.1.01404 *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 Unicode1416 * 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.01420 * @access private1421 *1422 * @link https://dev.mysql.com/doc/refman/8.0/en/identifiers.html1423 *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 /**1432 1366 * Prepares a SQL query for safe execution. 1433 1367 * 1434 1368 * Uses sprintf()-like syntax. The following placeholders can be used in the query string: … … 1436 1370 * - %d (integer) 1437 1371 * - %f (float) 1438 1372 * - %s (string) 1439 * - %i (identifier, e.g. table/field names)1440 1373 * 1441 1374 * All placeholders MUST be left unquoted in the query string. A corresponding argument 1442 1375 * MUST be passed for each placeholder. … … 1469 1402 * @since 5.3.0 Formalized the existing and already documented `...$args` parameter 1470 1403 * by updating the function signature. The second parameter was changed 1471 1404 * 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 uses1475 * `%d` and `$i` as a signed integer, whereas PHP only supports `%d`.1476 1405 * 1477 1406 * @link https://www.php.net/sprintf Description of syntax. 1478 1407 * … … 1504 1433 ); 1505 1434 } 1506 1435 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 1507 1458 /* 1508 1459 * Specify the formatting allowed in a placeholder. The following are allowed: 1509 1460 * … … 1524 1475 */ 1525 1476 $query = str_replace( "'%s'", '%s', $query ); // Strip any existing single quotes. 1526 1477 $query = str_replace( '"%s"', '%s', $query ); // Strip any existing double quotes. 1478 $query = preg_replace( '/(?<!%)%s/', "'%s'", $query ); // Quote the strings, avoiding escaped strings like %%s. 1527 1479 1528 // Escape any unescaped percents (i.e. anything unrecognised). 1529 $query = preg_replace( "/%(?:%|$|(?!($allowed_format)?[sdfFi]))/", '%%\\1', $query ); 1480 $query = preg_replace( "/(?<!%)(%($allowed_format)?f)/", '%\\2F', $query ); // Force floats to be locale-unaware. 1530 1481 1531 // Extract placeholders from the query. 1532 $split_query = preg_split( "/(^|[^%]|(?:%%)+)(%(?:$allowed_format)?[sdfFi])/", $query, -1, PREG_SPLIT_DELIM_CAPTURE ); 1482 $query = preg_replace( "/%(?:%|$|(?!($allowed_format)?[sdF]))/", '%%\\1', $query ); // Escape any unescaped percents. 1533 1483 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 ); 1484 // Count the number of valid placeholders in the query. 1485 $placeholders = preg_match_all( "/(^|[^%]|(%%)+)%($allowed_format)?[sdF]/", $query, $matches ); 1540 1486 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 }1618 1619 1487 $args_count = count( $args ); 1620 1488 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. 1627 1492 wp_load_translations_early(); 1628 1493 _doing_it_wrong( 1629 1494 'wpdb::prepare', … … 1644 1509 sprintf( 1645 1510 /* translators: 1: Number of placeholders, 2: Number of arguments passed. */ 1646 1511 __( '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, 1648 1513 $args_count 1649 1514 ), 1650 1515 '4.8.3' … … 1654 1519 * If we don't have enough arguments to match the placeholders, 1655 1520 * return an empty string to avoid a fatal error on PHP 8. 1656 1521 */ 1657 if ( $args_count < $placeholder _count) {1658 $max_numbered_placeholder = 0;1522 if ( $args_count < $placeholders ) { 1523 $max_numbered_placeholder = ! empty( $matches[3] ) ? max( array_map( 'intval', $matches[3] ) ) : 0; 1659 1524 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 }1668 1669 1525 if ( ! $max_numbered_placeholder || $args_count < $max_numbered_placeholder ) { 1670 1526 return ''; 1671 1527 } … … 1673 1529 } 1674 1530 } 1675 1531 1676 $args_escaped = array(); 1532 array_walk( $args, array( $this, 'escape_by_ref' ) ); 1533 $query = vsprintf( $query, $args ); 1677 1534 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 );1705 1706 1535 return $this->add_placeholder_escape( $query ); 1707 1536 } 1708 1537 … … 3950 3779 * @since 2.7.0 3951 3780 * @since 4.1.0 Added support for the 'utf8mb4' feature. 3952 3781 * @since 4.6.0 Added support for the 'utf8mb4_520' feature. 3953 * @since 6.1.0 Added support for the 'identifier_placeholders' feature.3954 3782 * 3955 3783 * @see wpdb::db_version() 3956 3784 * 3957 3785 * @param string $db_cap The feature to check for. Accepts 'collation', 'group_concat', 3958 * 'subqueries', 'set_charset', 'utf8mb4', 'utf8mb4_520', 3959 * or 'identifier_placeholders'. 3960 * @return bool True when the database feature is supported, false otherwise. 3786 * 'subqueries', 'set_charset', 'utf8mb4', or 'utf8mb4_520'. 3787 * @return bool True when the database feature is supported, false otherwise 3961 3788 */ 3962 3789 public function has_cap( $db_cap ) { 3963 3790 $db_version = $this->db_version(); … … 4001 3828 } 4002 3829 case 'utf8mb4_520': // @since 4.6.0 4003 3830 return version_compare( $db_version, '5.6', '>=' ); 4004 case 'identifier_placeholders': // @since 6.1.04005 /*4006 * As of WordPress 6.1, wpdb::prepare() supports identifiers via '%i',4007 * e.g. table/field names.4008 */4009 return true;4010 3831 } 4011 3832 4012 3833 return false; -
tests/phpunit/tests/db.php
494 494 $this->assertTrue( $wpdb->has_cap( 'collation' ) ); 495 495 $this->assertTrue( $wpdb->has_cap( 'group_concat' ) ); 496 496 $this->assertTrue( $wpdb->has_cap( 'subqueries' ) ); 497 $this->assertTrue( $wpdb->has_cap( 'identifier_placeholders' ) );498 497 $this->assertTrue( $wpdb->has_cap( 'COLLATION' ) ); 499 498 $this->assertTrue( $wpdb->has_cap( 'GROUP_CONCAT' ) ); 500 499 $this->assertTrue( $wpdb->has_cap( 'SUBQUERIES' ) ); 501 $this->assertTrue( $wpdb->has_cap( 'IDENTIFIER_PLACEHOLDERS' ) );502 500 $this->assertSame( 503 501 version_compare( $wpdb->db_version(), '5.0.7', '>=' ), 504 502 $wpdb->has_cap( 'set_charset' ) … … 1512 1510 } 1513 1511 1514 1512 /** 1513 * @ticket 56933 1514 * 1515 * @covers wpdb::prepare 1516 */ 1517 public function test_prepare_with_placeholders() { 1518 global $wpdb; 1519 1520 $sql = $wpdb->prepare( "'%%%s%%'", 'hello' ); 1521 $expected = '}hello{'; 1522 1523 $this->assertStringContainsString( $expected, $sql ); 1524 } 1525 1526 /** 1515 1527 * @dataProvider data_prepare_with_placeholders 1516 1528 */ 1517 1529 public function test_prepare_with_placeholders_and_individual_args( $sql, $values, $incorrect_usage, $expected ) { … … 1723 1735 false, 1724 1736 "'hello' 'foo##'", 1725 1737 ), 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 1738 ); 1823 1739 } 1824 1740 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.NotPrepared1836 $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.NotPrepared1842 $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 1847 }1848 1849 1741 /** 1850 1742 * @dataProvider data_escape_and_prepare 1851 1743 */