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

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.

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 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.

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.

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…

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.

[…] 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. […]

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.

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.