Among other issues, the code was vulnerable to SQL Injection.
SQL Injection is one of the easiest hacks to do. It's also one of the easiest to protect against. Still, it's a pretty common vulnerability! As much as I would love to have a backdoor into tons of sites, I can't help but lose sleep over this problem being out there. I have to share a few solutions.
What is SQL Injection?
For an example, let's take a very simple login page. I won't post the code, but the basic idea is to take a username and password, compare it to what's in the database, and log them in if it's a match. Easy peasy!
Here's the SQL to match it up in the database:
SELECT id FROM users WHERE username='bob' AND password='1234'
If it doesn't return anything, the username and password is wrong. If it finds a match, it returns the user's ID.
What if we change the username to bob'--?
SELECT id FROM users WHERE username='bob'--' AND password='1234'
Whoah! We just made it so that we don't even need the password! We can log in as anybody so long as we know their username!
The problems don't stop there. We can change a page that displays a list of items to sort/filter however we wish. We can increase the number of results returned and use that to do a very good DOS attack.
If the webhost is particularly unlucky, we can even execute SQL for other websites that they host -- even if those sites aren't vulnerable! (Hint: "USE abcDatabase;")
How do we protect against it?
That's easy. Sanitize your inputs!
Way 1 - Escaping data
You can properly escape the data before putting it into the SQL query.
Most languages have a function to escape out harmful characters, and it's usually pretty easy to use. This post is language-agnostic, but the basic idea in most languages is to pass in a potentially harmful string and get back a properly escaped one. The SQL query becomes the following:
SELECT id FROM users WHERE username='bob''--' AND password='1234'
As you can see, the SQL injection attempt will now be unsuccessful!
This time.
Watch out!
What if we have the following query that deletes the message with a given ID?
DELETE FROM messages WHERE messageId=12
It looks pretty harmless. Of course, we'll be sanitizing any user input anyways, right?
What if we give our code "1 OR 1=1" as the ID to delete? There are no special characters to escape. The string will get substituted as-is. What does this do to our query?
What if we give our code "1 OR 1=1" as the ID to delete? There are no special characters to escape. The string will get substituted as-is. What does this do to our query?
DELETE FROM messages WHERE messageId=1 OR 1=1
Oh, it just deletes everything. Even though we escaped all of the harmful characters, we are still vulnerable to SQL injection!
The quick solution is to put single quotes around every "variable" you're substituting in. It will work, but then you might run into weird logic errors when doing comparisons. A better solution is to use parameterized queries. Read on.
Way 2 - Parameterized Queries
Parameterized queries will properly escape any data, will verify that the type of data matches up, and will (hopefully) make sure everything's dandy with the character encodings. They are the best way to do SQL queries.
SELECT id FROM users WHERE username=@username AND password=@password; DELETE FROM messages WHERE messageId=@messageId;
It might look a little different in different languages, but that's the basic idea.
Now you need the code that puts in the data. This depends a lot on the language, but the basic idea is to say what type of data each parameter is and then to set it to a value. In VB.Net, here is the code for the second query:
Dim query = "DELETE FROM messages WHERE messageId=@messageId" Dim cmd As New SqlCommand(query, connection) cmd.Parameters.Add("@messageId", SqlDbType.Int) cmd.Parameters("@messageId").Value = 12 cmd.ExecuteNonQuery()
Parameterized queries are more verbose and somewhat confusing at first, but do use them. Use parameterized queries. Use parameterized queries.
Conclusion and Final Thoughts
SQL Injection is very easy to do, very common, and yet very easy to protect against. The simple solution is to just use parameterized queries everywhere. This OWASP page has some handy code snippets for your reference.
Here's a rule: Always use parameterized queries. Never insert your data directly into a query string (even if properly validated and escaped first). I don't care what your teacher or textbook says. Use parameterized queries. The only exception to this rule is when you know the risks and make an informed decision to go another route.