Changeset 53575 for trunk/tests/phpunit/tests/db.php

Timestamp:

06/24/2022 08:33:56 PM (3 years ago)

Author:

davidbaumwald

Message:

Database: Add %i placeholder support to $wpdb->prepare to escape table and column names.

WordPress does not currently provide an explicit method for escaping SQL table and column names. This leads to potential security vulnerabilities, and makes reviewing code for security unnecessarily difficult. Also, static analysis tools also flag the queries as having unescaped SQL input.

Tables and column names in queries are usually in-the-raw, since using the existing %s will straight quote the value, making the query invalid.

This change introduces a new %i placeholder in $wpdb->prepare to properly quote table and column names using backticks.

Props tellyworth, iandunn, craigfrancis, peterwilsoncc, johnbillion, apokalyptik.
Fixes #52506.

File:

• trunk/tests/phpunit/tests/db.php (modified) (2 diffs)

trunk/tests/phpunit/tests/db.php

@@ -493 +493 @@
         $this->assertTrue( $wpdb->has_cap( 'group_concat' ) );
         $this->assertTrue( $wpdb->has_cap( 'subqueries' ) );
+        $this->assertTrue( $wpdb->has_cap( 'identifier_placeholders' ) );
         $this->assertTrue( $wpdb->has_cap( 'COLLATION' ) );
         $this->assertTrue( $wpdb->has_cap( 'GROUP_CONCAT' ) );
         $this->assertTrue( $wpdb->has_cap( 'SUBQUERIES' ) );
+        $this->assertTrue( $wpdb->has_cap( 'IDENTIFIER_PLACEHOLDERS' ) );
         $this->assertSame(
             version_compare( $wpdb->db_version(), '5.0.7', '>=' ),
@@ -1718 +1720 @@
                 "'hello' 'foo##'",
             ),
-        );
+            array(
+                'SELECT * FROM %i WHERE %i = %d;',
+                array( 'my_table', 'my_field', 321 ),
+                false,
+                'SELECT * FROM `my_table` WHERE `my_field` = 321;',
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( 'evil_`_field', 321 ),
+                false,
+                'WHERE `evil_``_field` = 321;', // To quote the identifier itself, then you need to double the character, e.g. `a``b`.
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( 'evil_````````_field', 321 ),
+                false,
+                'WHERE `evil_````````````````_field` = 321;',
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( '``evil_field``', 321 ),
+                false,
+                'WHERE `````evil_field````` = 321;',
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( 'evil\'field', 321 ),
+                false,
+                'WHERE `evil\'field` = 321;',
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( 'evil_\``_field', 321 ),
+                false,
+                'WHERE `evil_\````_field` = 321;',
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( 'evil_%s_field', 321 ),
+                false,
+                "WHERE `evil_{$wpdb->placeholder_escape()}s_field` = 321;",
+            ),
+            array(
+                'WHERE %i = %d;',
+                array( 'value`', 321 ),
+                false,
+                'WHERE `value``` = 321;',
+            ),
+            array(
+                'WHERE `%i = %d;',
+                array( ' AND evil_value', 321 ),
+                false,
+                'WHERE `` AND evil_value` = 321;', // Won't run (SQL parse error: "Unclosed quote").
+            ),
+            array(
+                'WHERE %i` = %d;',
+                array( 'evil_value -- ', 321 ),
+                false,
+                'WHERE `evil_value -- `` = 321;', // Won't run (SQL parse error: "Unclosed quote").
+            ),
+            array(
+                'WHERE `%i`` = %d;',
+                array( ' AND true -- ', 321 ),
+                false,
+                'WHERE `` AND true -- ``` = 321;', // Won't run (Unknown column '').
+            ),
+            array(
+                'WHERE ``%i` = %d;',
+                array( ' AND true -- ', 321 ),
+                false,
+                'WHERE ``` AND true -- `` = 321;', // Won't run (SQL parse error: "Unclosed quote").
+            ),
+            array(
+                'WHERE %2$i = %1$d;',
+                array( '1', 'two' ),
+                false,
+                'WHERE `two` = 1;',
+            ),
+            array(
+                'WHERE \'%i\' = 1 AND "%i" = 2 AND `%i` = 3 AND ``%i`` = 4 AND %15i = 5',
+                array( 'my_field1', 'my_field2', 'my_field3', 'my_field4', 'my_field5' ),
+                false,
+                '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).
+            ),
+            array(
+                'WHERE id = %d AND %i LIKE %2$s LIMIT 1',
+                array( 123, 'field -- ', false ),
+                true, // Incorrect usage.
+                null, // Should be rejected, otherwise the `%1$s` could use Identifier escaping, e.g. 'WHERE `field -- ` LIKE field --  LIMIT 1' (thanks @vortfu).
+            ),
+            array(
+                'WHERE %i LIKE %s LIMIT 1',
+                array( "field' -- ", "field' -- " ),
+                false,
+                "WHERE `field' -- ` LIKE 'field\' -- ' LIMIT 1", // In contrast to the above, Identifier vs String escaping is used.
+            ),
+        );
+    }
+
+    public function test_allow_unsafe_unquoted_parameters() {
+        global $wpdb;
+
+        $sql    = 'WHERE (%i = %s) OR (%10i = %10s) OR (%5$i = %6$s)';
+        $values = array( 'field_a', 'string_a', 'field_b', 'string_b', 'field_c', 'string_c' );
+
+        $default = $wpdb->allow_unsafe_unquoted_parameters;
+
+        $wpdb->allow_unsafe_unquoted_parameters = true;
+
+        // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
+        $part = $wpdb->prepare( $sql, $values );
+        $this->assertSame( 'WHERE (`field_a` = \'string_a\') OR (`   field_b` =   string_b) OR (`field_c` = string_c)', $part ); // Unsafe, unquoted parameters.
+
+        $wpdb->allow_unsafe_unquoted_parameters = false;
+
+        // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
+        $part = $wpdb->prepare( $sql, $values );
+        $this->assertSame( 'WHERE (`field_a` = \'string_a\') OR (`   field_b` = \'  string_b\') OR (`field_c` = \'string_c\')', $part );
+
+        $wpdb->allow_unsafe_unquoted_parameters = $default;
+
     }