WordPress Plugin Review Team → Unsafe SQL calls

[remcotolsma]

We received the following feedback from the WordPress Plugin Review Team about a plugin that ships Action Scheduler library.

## Unsafe SQL calls

When making database calls, it's highly important to protect your code from SQL injection vulnerabilities. You need to update your code to use wpdb calls and prepare() with your queries to protect them.

Please review the following:

• https://developer.wordpress.org/reference/classes/wpdb/#protect-queries-against-sql-injection-attacks
• https://codex.wordpress.org/Data_Validation#Database
• https://make.wordpress.org/core/2012/12/12/php-warning-missing-argument-2-for-wpdb-prepare/
• https://ottopress.com/2013/better-know-a-vulnerability-sql-injection/

Example(s) from your plugin:

packages/woocommerce/action-scheduler/classes/data-stores/ActionScheduler_DBStore.php:289 $group_id = (int) $wpdb->get_var( $wpdb->prepare( "SELECT group_id FROM {$wpdb->actionscheduler_groups} WHERE slug=%s", $slug ) );
# There is a call to a wpdb::prepare() function, that's correct.
# You cannot add variables like "$wpdb->actionscheduler_groups" directly to the SQL query.
# Using wpdb::prepare($query, $args) you will need to include placeholders for each variable within the query and include the variables in the second parameter.


packages/woocommerce/action-scheduler/classes/data-stores/ActionScheduler_DBLogger.php:58 $entry = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->actionscheduler_logs} WHERE log_id=%d", $entry_id ) );
# There is a call to a wpdb::prepare() function, that's correct.
# You cannot add variables like "$wpdb->actionscheduler_logs" directly to the SQL query.
# Using wpdb::prepare($query, $args) you will need to include placeholders for each variable within the query and include the variables in the second parameter.

... out of a total of 36 incidences.

Note: Passing individual values to wpdb::prepare using placeholders is fairly straightforward, but what if we need to pass an array of values instead?

You'll need to create a placeholder for each item of the array and pass all the corresponding values to those placeholders, this seems tricky, but here is a snippet to do so.

$wordcamp_id_placeholders = implode( ', ', array_fill( 0, count( $wordcamp_ids ), '%d' ) );
$prepare_values = array_merge( array( $new_status ), $wordcamp_ids );
$wpdb->query( $wpdb->prepare( "
        UPDATE `$table_name`
        SET `post_status` = %s
        WHERE ID IN ( $wordcamp_id_placeholders )",
        $prepare_values
) );

There is a core ticket that could make this easier in the future: https://core.trac.wordpress.org/ticket/54042

Example(s) from your plugin:

packages/woocommerce/action-scheduler/classes/abstracts/ActionScheduler_Abstract_ListTable.php:488 $columns = '`' . implode( '`, `', $this->get_table_columns() ) . '`';
packages/woocommerce/action-scheduler/classes/abstracts/ActionScheduler_Abstract_ListTable.php:491 $where = 'WHERE (' . implode( ') AND (', $where ) . ')';

... out of a total of 3 incidences.

Previously reported similar issues:

• [Task] Codebase should pass linting checks #971
• WordPress.org plugin review warning/issue about internationalization #1011
• WordPress.org plugin review warnings/issues #1027
• WordPress.org plugin review warnings/issues 🔁 3️⃣ #1068
• WordPress Plugin Review Team → Determine files and directories locations correctly #1256