We recently had a strange interaction with the team running the WordPress Plugin Directory over their failure to make sure a likely exploited vulnerability was fixed. It was yet another example of their poor handling of security. That runs counter to their own stated expectations:

All members of the plugin team are held to an exceptionally high standard, not just in their ability to process code for security, but also in the way they handle security issues, ethical/behavioral situations, and privileged information.

The problems continue.

Two weeks ago, the plugin Optimize Database after Deleting Revisions was re-opened on the Plugin Directory without the known vulnerability in it being fixed. In speaking to the new owner of the plugin, we found out that the Plugin Directory team were not concerned about that vulnerability, but the handling of SQL statements in the plugin. We are not aware of any claim of a SQL injection vulnerability in the plugin that would lead to that focus, but the larger problem is that they somehow managed to miss that the change made to address their concern is still insecure in what should be a fairly obvious way.

The documentation for the esc_sql() escaping function has this important note (emphasis in the original) about using it:

Be careful in using this function correctly. It will only escape values to be used in strings in the query. That is, it only provides escaping for values that will be within quotes in the SQL (as in field = '{$escaped_value}'). If your value is not going to be within quotes, your code will still be vulnerable to SQL injection. For example, this is vulnerable, because the escaped value is not surrounded by quotes in the SQL query: ORDER BY {$escaped_value}. As such, this function does not escape unquoted numeric values, field names, or SQL keywords.

So what you shouldn’t see is usage of the function without quotes around the output. Yet in the updated version of the plugin, you find that repeatedly. Here, for example, is the first instance of it being added in the new version of the plugin (the double quotes are part of generating a string, not quotes in the string as would be needed):

1016

FROM " . esc_sql( $prefix . 'posts' ) . " p1, " . esc_sql( $prefix . 'posts' ) . " p2

That came as part of a larger change being made to use $wpdb->prepare() instead of sprinf() when crafting SQL statements. Simply using $wpdb->prepare() doesn’t actually solve problems, as we found with a plugin with 300,000+ installs recently. So they need to make sure it is being used properly, but they didn’t with this plugin.

Considering the Plugin Directory team’s open hostility to us trying to help them address a different problem with their handling of security, we don’t know what can be done here. They have a serious problem handling security and simply deny it, as the problems continue.