Skip to content

Add DirectDB Security Check for Database Query Escaping #1075

@davidperezgar

Description

@davidperezgar

Overview

Add a security check to detect unsafe direct database queries that use unescaped parameters. This check would identify SQL injection vulnerabilities in WordPress plugins by analyzing $wpdb method calls for properly escaped parameters.

What This Check Does

The DirectDB check (inspired by wporg-code-analysis's DirectDBSniff) detects database queries that contain unescaped user input or variables that could lead to SQL injection vulnerabilities. It's a context-aware sniff that tracks variable assignments and escaping throughout the code to determine if data is properly sanitized before being used in SQL queries.

Example of Unsafe Code

// UNSAFE: Unescaped variable in query
function insecure_query( $foo ) {
    global $wpdb;
    $wpdb->query( "SELECT * FROM $wpdb->posts WHERE foo = '$foo'" );
}

// UNSAFE: Superglobal directly in query
function insecure_query_2() {
    global $wpdb;
    $wpdb->query( "SELECT * FROM $wpdb->users WHERE id = '" . $_POST['user_id'] . "'" );
}

// UNSAFE: Non-SQL-safe function (sanitize_text_field is NOT SQL-safe)
function insecure_query_3() {
    global $wpdb;
    $bar = sanitize_text_field( $_POST['value'] );
    $wpdb->query( "SELECT * FROM $wpdb->posts WHERE title = '$bar'" );
}

// UNSAFE: Variable in $wpdb->prepare() template
function insecure_query_4() {
    global $wpdb;
    $bar = $_POST['value'];
    $wpdb->query( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE title='$bar' AND status=%s", $status ) );
}

Example of Safe Code

// SAFE: Using esc_sql()
function secure_query_1( $foo ) {
    global $wpdb;
    $wpdb->query( "SELECT * FROM $wpdb->posts WHERE foo = '" . esc_sql( $foo ) . "'" );
}

// SAFE: Using $wpdb->prepare()
function secure_query_2( $foo ) {
    global $wpdb;
    $wpdb->query( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE foo = %s", $foo ) );
}

// SAFE: Using absint() for numeric values
function secure_query_3( $user_id ) {
    global $wpdb;
    $wpdb->query( "SELECT * FROM $wpdb->users WHERE id = " . absint( $user_id ) );
}

// SAFE: Using $wpdb helper methods with built-in escaping
function secure_query_4( $data ) {
    global $wpdb;
    $wpdb->update( $wpdb->posts, $data, array( 'ID' => $post_id ) );
}

// SAFE: Variable assignment tracking
function secure_query_5( $foo ) {
    global $wpdb;
    $escaped_foo = esc_sql( $foo );
    $wpdb->query( "SELECT * FROM $wpdb->posts WHERE foo = '$escaped_foo'" );
}

How It Works

The DirectDB check:

  1. Listens for $wpdb method calls (query, get_var, get_col, get_row, get_results)

  2. Tracks variable assignments throughout the code to understand if values have been escaped

  3. Identifies SQL-safe escaping functions:

    • esc_sql()
    • absint(), intval(), floatval()
    • like_escape()
    • wp_parse_id_list()
    • sanitize_sql_orderby()
    • And more...
  4. Recognizes NOT-safe functions that developers often mistake for SQL escaping:

    • sanitize_text_field() - NOT SQL-safe!
    • sanitize_title(), sanitize_key() - NOT SQL-safe!
    • addslashes(), addcslashes() - NOT SQL-safe!
    • esc_attr(), esc_html() - NOT SQL-safe!
    • filter_input() - NOT SQL-safe!
  5. Recognizes safe $wpdb methods with built-in escaping:

    • $wpdb->insert()
    • $wpdb->update()
    • $wpdb->delete()
    • $wpdb->replace()
  6. Provides detailed error messages showing the chain of variable assignments that led to the unsafe query

  7. Reduces false positives by:

    • Understanding ternary operators with literal values
    • Recognizing when variables are always assigned literals
    • Tracking escaping through foreach loops
    • Handling array_map() and array_walk() with escaping functions
    • Warning instead of erroring for table/column names (which often can't be escaped)
    • Recognizing implicit safe functions (gmdate, count, md5, etc.)
  8. Differentiates errors from warnings:

    • Errors: Unescaped user input, superglobals, function parameters
    • Warnings: Table names, column names, $this properties, CREATE TABLE queries

Implementation Approach

Port DirectDBSniff from wporg-code-analysis

Port the existing DirectDBSniff and AbstractEscapingCheckSniff from the wporg-code-analysis plugin.

Files to port:

  • MinimalPluginStandard/Sniffs/DirectDBSniff.php
  • MinimalPluginStandard/AbstractEscapingCheckSniff.php
  • MinimalPluginStandard/AbstractSniffHelper.php

Tests:

  • DirectDBUnitTest.php
  • DirectDBUnitTest.php-bad.inc
  • DirectDBUnitTest.php-safe.inc

Metadata

Metadata

Assignees

Labels

ChecksAudit/test of the particular part of the plugin[Team] PluginsIssues owned by Plugins Team

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions