We get a lot of submissions to the WordPress.org plugin repository, and so there is often a lot of dangerous code submitted. Usually this isn’t malicious, it’s just by people who honestly don’t know that their code has problems. Understanding those problems is the first step to fixing them.

So here’s one common vulnerability we see in code submissions a lot: SQL Injection

To understand SQL Injection, let’s quote Wikipedia for a moment:

SQL injection is a code injection technique, used to attack data driven applications, in which malicious SQL statements are inserted into an entry field for execution

Here’s a piece of code made for WordPress, which is querying the database for a post:

// bad code, do not use
$results = $wpdb->get_results( "SELECT * FROM $wpdb->posts WHERE ID = $id" );

If you don’t see the problem with this code right away, then you should continue reading this post.

(Yes, this article shows the basics of the prepare() function. If you already know about the prepare() function, you might be shocked at the number of people who do not.)

The problem with SQL Injection vulnerabilities is that sometimes they can be hard to spot. The issue with the above code is actually context-dependent. The question you must answer is “What are the possible contents of the $id variable?”.

If $id = 123, then all is well.

But, if it is at all possible for $id = “-1; SELECT * from wp_users;” then you might have a real problem.

Sometimes we see code like this in submitted plugins:

// bad code, do not use
$results = $wpdb->get_results( "SELECT * FROM $wpdb->posts WHERE ID = ". $_GET['id'] );

Now, we have no idea what “id” contains, and in fact, we’re leaving that entirely up to the visitor of the site. Or the hacker of the site, in this case.

There should be no case where user-input can make it into an SQL statement without being first checked for sanity.

Sometimes, that check is easy. In this case, the ID should always be a number. So we can secure the query like so:

// kinda bad code, still, do not use
$id = (int) $_GET['id'];
$results = $wpdb->get_results( "SELECT * FROM $wpdb->posts WHERE ID = $id" );

This is relatively safe, but not the recommended solution. It’s the naive approach, because we’re thinking that hey, we can just check the value ourselves and handle it accordingly. For integers, sure, but for more complex cases we can’t. Sometimes we can’t even do it for integers, so it’s best to avoid this sort of thinking entirely.

Don’t try to sanitize your inputs to SQL functions yourself. Let the sanitization functions do it for you. WordPress includes a function called prepare() to handle this safely.

The right way:

// good code
$results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d", $id ) );

The prepare() function goes through a number of various checks and such, but eventually it ends up replacing the %d with the integer value of $id. Sure you could have done that yourself, but with prepare, you don’t have to think about how it’s doing it.

Because what if $id wasn’t an integer? If it’s a string, then prepare eventually ends up calling a function named mysql_real_escape_string(). This is a core PHP function that does the necessary escaping for you. It needs to always be called for data inputs to SQL, so the upshot is that you must always use prepare.

That bit is important, so read it again: Whenever you make a direct SQL call, any variable inputs to that query must go through a prepare() cycle. Not sometimes, always 1.

Here’s another example using a string:

// good code
$results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM $wpdb->postmeta WHERE meta_key = %s", $metakey ) );

I’m selecting all the rows from the postmeta table with a specific meta key. Note that meta_key is a string, so I used the %s instead of the %d (string vs decimal). Because of this, prepare will take care of the proper quoting of the string for us, we don’t need (or want) to add quotes around it ourselves.

Given this, no matter what $metakey is, it should be safe and properly escaped. SQL Injection should not be possible, barring a bug deep in the mysql library itself or some sort of configuration error. It’s as safe as we can reasonably make it, and certainly safer than any code we try to write ourselves to handle sanitizing it for SQL.

You can use more than one argument if needed:

// good code
$results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM $wpdb->postmeta WHERE meta_key = %s and meta_value = %s", $metakey, $metavalue ) );

So that’s SQL Injection and how to avoid it. Just always use prepare().

See how simple that is?

So go forth, examine your SQL, and please make sure you prepare’d it properly. Let’s reduce the amount of bad plugin code out there. And if you find plugin authors not using prepare(), email them about it. But be nice, they probably just didn’t know.


1. Rules are there so that we think before we break them for special cases…

Shortlink:

18 Comments

  1. Are there any plugins that will help with picking up vulnerable code and sql stuff or can that not be done? I’m thinking of something like the theme check plugin.

    • It would be possible to write code to detect a potential case, but there’s so many ways of writing code that you couldn’t really get them all. Also, the false positive rate would be high enough to make such a scan rather pointless. There are certainly tools that could help, but really nothing beats education about the possible issues.

  2. […] Understand how to prevent SQL injection vulnerabilities in WordPress → […]

  3. […] Better Know a Vulnerability: SQL Injection – We get a lot of submissions to the WordPress.org plugin repository, and so there is often a lot of dangerous code submitted. Usually this isn’t malicious, it’s just by people who honestly don’t know that their code has problems. […]

  4. Key to avoiding sql injection is to know the possible ways that a vulnerability can be exploited. You can go through the below post for more additional info on it.
    http://digitalab.org/2013/05/sql-injection/

  5. Thank you for the information. I’ve read a handful of other articles about SQL injection, but I never quite got the grasp of it. You made it clear to understand and I’ll make sure to “prepare” my code from now on.

    Thanks again!

  6. […] Best to be aware of this WordPress Vulnerability: SQL Injection Read up on this common vulnerability and how you can make your site even more hacker-proof… […]

  7. Oops, this is little scary, i think. Must do revision my plugins 🙁

  8. Thanks. Always use prepare() .. 😉

  9. These are all examples of querying the database. How can you be sure to prevent injection when you are using $wpdb->insert?

    • SQL Injection can only happen when you are directly dealing with SQL statements yourself.

      If you’re using the insert() and similar functionality in the wpdb class, then it does the SQL statement handling for you, including prepare’ing statements and such. You don’t have to deal with it at all in that case, it’s automatically handled.

  10. Thanks a lot. I like the way you write, it is impossible not to understand it 🙂

    The plugins and Themes at the WordPress.org repository, are they safe or can they have this SQL issue?

  11. Thanks Otto…!
    I think this is the nice post. I got pretty much knowledge about wordpress queriy’s behavior. Thanks again for sharing this knowledge.

  12. #SQL Injection Prevent code

    RewriteEngine On
    RewriteBase /
    RewriteCond %{REQUEST_METHOD} ^(HEAD|TRACE|DELETE|TRACK) [NC]
    RewriteRule ^(.*)$ – [F,L]
    RewriteCond %{QUERY_STRING} \.\.\/ [NC,OR]
    RewriteCond %{QUERY_STRING} boot\.ini [NC,OR]
    RewriteCond %{QUERY_STRING} tag\= [NC,OR]
    RewriteCond %{QUERY_STRING} ftp\: [NC,OR]
    RewriteCond %{QUERY_STRING} http\: [NC,OR]
    RewriteCond %{QUERY_STRING} https\: [NC,OR]
    RewriteCond %{QUERY_STRING} (\|%3E) [NC,OR]
    RewriteCond %{QUERY_STRING} mosConfig_[a-zA-Z_]{1,21}(=|%3D) [NC,OR]
    RewriteCond %{QUERY_STRING} base64_encode.*\(.*\) [NC,OR]
    RewriteCond %{QUERY_STRING} ^.*(\[|\]|\(|\)||ê|”|;|\?|\*|=$).* [NC,OR]
    RewriteCond %{QUERY_STRING} ^.*("|'|<|>|\|{||).* [NC,OR]
    RewriteCond %{QUERY_STRING} ^.*(%24&x).* [NC,OR]
    RewriteCond %{QUERY_STRING} ^.*(%0|%A|%B|%C|%D|%E|%F|127\.0).* [NC,OR]
    RewriteCond %{QUERY_STRING} ^.*(globals|encode|localhost|loopback).* [NC,OR]
    RewriteCond %{QUERY_STRING} ^.*(request|select|insert|union|declare).* [NC]
    RewriteCond %{HTTP_COOKIE} !^.*wordpress_logged_in_.*$
    RewriteRule ^(.*)$ – [F,L]

  13. Thanks for your post!
    There is a way to prepare data for $wpdb with $mq = $meta_query->get_sql(‘post’, $wpdb->posts, ‘ID’) ?

Leave a Reply

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>

Need to post PHP code? Wrap it in [php] and [/php] tags.