Skip to content

Use %i identifier placeholder in WordPress query strategies instead of interpolation #33

Description

@alexstandiford

Problem

The WordPress query strategies interpolate table/column identifiers directly into SQL instead of using the %i identifier placeholder:

  • QueryStrategy::estimatedCount(): $wpdb->get_var("SELECT COUNT(*) FROM " . $table->getName()) — no escaping at all.
  • TableExistsStrategy, TableCreate/Delete/UpdateStrategy, CanQueryWordPressDatabase: DROP TABLE IF EXISTS $tableName, CREATE TABLE ... {$table->getName()}, WHERE TABLE_NAME = '{$tableName}' followed by a prepare() call with zero value args (which also triggers _doing_it_wrong).

Today the identifiers resolve to globalPrefix + localPrefix + a hardcoded literal, so this is not currently injectable. But it relies on every caller's prefix/name being a trusted constant — a future caller that derives a table name or prefix dynamically would expose injection, and the prepare()-with-no-placeholders calls are already incorrect usage.

Desired outcome

Use $wpdb->prepare() with %i for identifiers (WP 6.2+) throughout the WordPress query strategies, and remove the no-op prepare() calls that pass an already-interpolated string with no args.

Notes

Surfaced by the Siren WordPress plugin during WordPress.org review-simulation. Direct sibling of a finding the WordPress.org team raised against Siren (esc_sql concatenation in a COUNT query).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions