How vulnerable is my code to SQL injection?

Question

Ok I don't want this to be a hacking advice question, so please don't down-vote for that. I work in a web shop and I've found some of our old PHP pages are vulnerable to SQL injection in the username and want to know how bad.

We use a PHP string to embed the user input from the POST on the login form.

$uname = $_POST['username'];
$pass  = md5($_POST['pass']);
$sql = "SELECT * FROM users WHERE username='$uname' AND password='$pass' AND userlevel='user'";
...

then I run the query.

Now, I'm no SQL expert, I just use what I can piece together on phpMyAdmin. But I was able to log in without a username by instead using:

' OR 1 '

I know to escape the user input, I use mysql_real_escape_string.

My question is, how vulnerable is this code, and could someone log into this page and not need the password? I would think maybe they wouldn't need the username, but could only brute force the password. But I'm no SQL guru and am wondering if some tricks could be used against us.

We use MySQL.

And please I don't need any lectures on input validation, I know how bad this is. We should be doing lots of things, like timeouts and lockouts on our page so it can't be brute-forced.

Answer 1

"Could someone log into this page and not need the password": Yes, trivially. Try the username yourfavoriteadmin' OR 1; --.

May as well link this, since certainly somebody will...

Answer 2

try where $_POST['username'] is:

no one'; delete from users --

Answer 3

It’s very vulnerable. If you know about all the nifty stuff like mysql_real_escape_string why do you waste your time and ask this question? You should be all over that code, fixing it. You know, like, NOW.

Answer 4

Always clean up your data before using in any SQL query, if you're working with PHP >= 5.2, you have the Filter functions.

Simple question : why don't change the order WHERE pass = 'blabl' AND username = 'bla' ? Have you an index on this fields ?

Another question: what is the point with this line

$uname = $_POST['username'];

adding a variable, just for quoting/double-quoting effect in concatenation ? Without filtering nor escaping data ?

Answer 5

The code as written isn't technically vulnerable at all. Your SQL query contains variable $username but you never initialize it or set it to anything. That's a bug and you'll never get a valid result from MySQL.

Once you fix that bug, however, you should be escaping your variables with mysql_real_escape_string().

http://us.php.net/manual/en/function.mysql-real-escape-string.php