Changes between Initial Version and Version 38 of Ticket #54042
- Timestamp:
- 10/30/2022 12:25:20 PM (3 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #54042
- Property Keywords has-patch dev-feedback needs-testing early has-unit-tests added
-
Property
Version
changed from
trunk
to -
Property
Summary
changed from
Extending wpdb::prepare() to support table/field names, and IN() operator
toExtending wpdb::prepare() to support IN() operator
-
Property
Milestone
changed from
Awaiting Review
toFuture Release
-
Ticket #54042 – Description
initial v38 1 wpdb::prepare() h as really helped avoid SQL Injection vulnerabilities, ensuring most variables are escapedcorrectly.1 wpdb::prepare() helps avoid SQL Injection vulnerabilities, by escaping most variables correctly. 2 2 3 But it does not support table/field names, so developers need to implement their own escaping, which is not always done safely. [https://github.com/WordPress/WordPress/blob/988c8be693e0d12aeacae30f1d4bebfb98f7e5a0/wp-includes/wp-db.php#L2778 If this example was copied], with no other checks, there could be an issue if `$table` included backtick or other invalid characters: 3 WP 6.1 added support for Identifiers (table/field names) with `%i`, in #52506. 4 4 5 {{{#!php 6 <?php 7 $table_parts = explode( '.', $table ); 8 $table = '`' . implode( '`.`', $table_parts ) . '`'; // INSECURE? 9 }}} 10 11 Likewise, it's also fairly common to make a mistake when including values with the `IN ()` operator, for example: 5 But it's also fairly common to make a mistake to include values with the `IN()` operator, for example: 12 6 13 7 {{{#!php … … 16 10 }}} 17 11 18 Developers need to be sure that`$ids` has come from a trusted source, or use something like `wp_parse_id_list()` or `array_map('intval', $ids)`.12 Developers need to be sure `$ids` has come from a trusted source, or use something like `wp_parse_id_list()` or `array_map('intval', $ids)`. 19 13 20 14 ---- 21 15 22 Considering the `wpdb::prepare()` documentation says it "Uses sprintf()-like syntax", could we add a couple of placeholders to safely include these values? e.g. 16 Maybe support could be added with: 23 17 24 18 {{{#!php 25 19 <?php 26 $wpdb->prepare(' SELECT * FROM %i WHERE ID IN (%...d)', $table, $ids);20 $wpdb->prepare('WHERE id IN (%...d)', $ids); 27 21 }}} 28 22 29 Where `% i` would be used for Identifiers (e.g. table/field names), where the value would be quoted with backticks, and invalid characters removed.23 Where `%...d` or `%...s` would safely (and easily) include a comma separated array of integers or strings - taking the idea of using '...' for variadics in PHP. 30 24 31 And `%...d` or `%...s` would safely (and easily) include a comma separated array of integers or strings - taking the idea of using '...' for variadics in PHP. 32 33 https://dev.mysql.com/doc/refman/8.0/en/identifiers.html 25 https://wiki.php.net/rfc/variadics 26 https://www.php.net/manual/en/functions.arguments.php#functions.variable-arg-list 34 27 https://dev.mysql.com/doc/refman/8.0/en/comparison-operators.html#operator_in