Make WordPress Core

Changes between Initial Version and Version 38 of Ticket #54042


Ignore:
Timestamp:
10/30/2022 12:25:20 PM (3 years ago)
Author:
craigfrancis
Comment:

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 to Extending wpdb::prepare() to support IN() operator
    • Property Milestone changed from Awaiting Review to Future Release
  • Ticket #54042 – Description

    initial v38  
    1 wpdb::prepare() has really helped avoid SQL Injection vulnerabilities, ensuring most variables are escaped correctly.
     1wpdb::prepare() helps avoid SQL Injection vulnerabilities, by escaping most variables correctly.
    22
    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:
     3WP 6.1 added support for Identifiers (table/field names) with `%i`, in #52506.
    44
    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:
     5But it's also fairly common to make a mistake to include values with the `IN()` operator, for example:
    126
    137{{{#!php
     
    1610}}}
    1711
    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)`.
     12Developers need to be sure `$ids` has come from a trusted source, or use something like `wp_parse_id_list()` or `array_map('intval', $ids)`.
    1913
    2014----
    2115
    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.
     16Maybe support could be added with:
    2317
    2418{{{#!php
    2519<?php
    26 $wpdb->prepare('SELECT * FROM %i WHERE ID IN (%...d)', $table, $ids);
     20$wpdb->prepare('WHERE id IN (%...d)', $ids);
    2721}}}
    2822
    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.
     23Where `%...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.
    3024
    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
     25https://wiki.php.net/rfc/variadics
     26https://www.php.net/manual/en/functions.arguments.php#functions.variable-arg-list
    3427https://dev.mysql.com/doc/refman/8.0/en/comparison-operators.html#operator_in