Security against SQL injection in Wordpress

I like Wordpress for its interface and ease of use, but it has a security problem that keeps cropping up again and again: vulnerability to SQL injection because of things like this:

$wpdb->query("SELECT row FROM table WHERE column = '$parameter'")
instead of
$wpdb->query("SELECT row FROM table WHERE column = '%s'", $parameter)

The latter is what Drupal uses, and it allows the query function to escape all the parameters appropriately before inserting them into the query string. The former is what Wordpress uses: the idea is that $parameter has hopefully been passed through the $wpdb->escape() function (which uses addslashes() to escape special SQL characters such as quotes) at some point earlier on.

In wp-comment.php, for example, there's a database query that uses the parameter $comment_author, which can originate from a parameter passed in as a trackback. In order to make sure that this parameter is safe to use in the query, the original $_POST[blog_name'] is

  1. Passed through stripslashes_deep() to remove the slashes that magic_quotes_gpc() has added if it's on by default [wp-settings.php].
  2. Passed through add_magic_quotes() to make sure quotes are escaped (add_magic_quotes calls $wpdb->escape, which calls addslashes()) [wp-settings.php].
  3. Passed through stripslashes again [wp-trackback.php].
  4. Converted to $blog_name [wp-trackback.php].
  5. Passed through mb_convert_encoding($blog_name, get_option('blog_charset'), $charset), to convert strings provided in a different character set into the local character set [wp-trackback.php].
  6. Passed through $wpdb->escape (addslashes()) again [wp-trackback.php].
  7. Converted to $comment_author [wp-trackback.php].
  8. Passed as part of the $commentdata array into wp_filter_comment(), which calls apply_filters(), which applies the pre_comment_author_name filter, which passes the string through strip_tags, trim, wp_filter_kses and wp_specialchars. [wp-comment.php]
  9. Finally, inserted into the database by direct insertion into a query as above.

There are two problems here: firstly, that whole chain above is fragile - it's hard to keep track of which filters the variable has passed through, so a change in one function could have unforeseen consequences - and secondly, the use of addslashes() rather than mysql_real_escape_string().

While addslashes does a naive replacement of a few special characters, and mysql_escape_string replaces a few more, neither take into account the character set used by the database table, which means that characters can sneak through unescaped. In contrast, mysql_real_escape_string checks the character set used by the database connection and performs the appropriate replacement using that character set.

There are multiple exploits for Wordpress based on the use of addslashes, and an issue in the Wordpress queue, but it still hasn't been fixed. Apparently mysql_real_escape_string was tried for a while but caused problems for people with certain character sets, so was backed out again, leaving anyone who uses the GBK or Big5 character sets vulnerable to SQL injection.

To test the exploit, you need to

  1. Compile MySQL using the --with-extra-charsets=all parameter.
  2. Change the appropriate database table to use the GBK character set: ALTER TABLE wp_table_name DEFAULT CHARACTER SET gbk;.
  3. Edit wp-config.php and set DB_CHARSET to 'GBK'.
  4. POST a crafted query with a parameter that contains the magic characters (generated in PHP using chr(0xbf) . chr(0x27))

I got this to work with Wordpress 2.5 and an INSERT statement in wp-includes/comment.php, so I'm dubious about Matt Mullenweg's claim that comments posted to Wordpress blogs are secure. While most blogs (those that don't use GBK or Big5) aren't vulnerable, many probably are. There's a limit to how much damage can be done, because you can't stack multiple queries in PHP's mysql_query function, so you can't tack a DELETE statement onto the end of an INSERT, for example. Even so, the use of "prepared" statements (they're not real server-side prepared statements) throughout Wordpress (I see several have been changed just today) would make things more secure.

By the way, any Drupal users who are feeling smug can have a look at this: the supposedly bulletproof kses filter has flaws that leave sites open to XSS; already worked around in Wordpress 2.5 but still open in Drupal as far as I know. Also, the "breaking backwards compatibility is a feature" mantra of Drupal sadly means "all your security audits are invalidated by a new version".