Take the 2-minute tour ×
Information Security Stack Exchange is a question and answer site for Information security professionals. It's 100% free, no registration required.

We have this legacy website which has horrendous code in it. I have just been looking through the login form/code and can see un-sanitised sql queries. E.g. in a nutshell:

$email=$_POST['email'];
$password = $_POST['password'];

$query = "SELECT aes_decrypt(password, 'asdasdasd') AS password FROM users WHERE email='$email'";
$result = mysql_query($query);
$row = mysql_fetch_array($result);

if($password == $row['password']){
    // set cookies and redirect user etc etc
}
else {
    // error not correct login details
}

My question is, I realise that this is bad because of SQL injection. And I know the basics of that. i.e. You could put '; DROP ALL TABLES; -- and that would "cause havoc".

Can anything more "interesting" or "useful" (to the potential hacker) be done? I.e., can you output a list of passwords, or download the tables with this? Or could you manage to log yourself in as a user of the site, for instance if you only know their email address?

Mainly for interest, but also for better insight (and therein finding better approaches to coding).

share|improve this question
4  
Should we point out that you should NEVER use AES to ENCRYPT a user's password? So even before we to what is wrong with the code posted and how it could be exploited you already have a security problem. –  Ramhound Jun 18 '12 at 12:56
1  
This legacy software is the BAIN of my life! I'm pretty glad that there is any encryption whatsoever. Variables are named $lalapoo. To get stuff out of a database there is first a query which gets all the ids, does a while($row=mysql_fetch_array($result)) to put all the ids into an array. Then does a foreach($lalapoo as $lala) and does another SELECT query for each id to get the info out. There are huge switch statements where the same 100 or so lines of code are repeated for each case with 1 tiny difference in each. Honestly, AES encryption is one of the few "positive" bits! –  Thomas Clayson Jun 18 '12 at 13:07
    
Looks like you have a very tough job ahead of you. –  Terry Chia Jun 18 '12 at 13:08
2  
Ahahah, who the hell wrote that code? :D I love your comment @ThomasClayson ! It seems that it has been written in .. what ? 1870? –  Cyril N. Jun 18 '12 at 13:10
2  
It wouldn't hurt to also make sure the query returns exactly one record. –  Mark Burnett Jun 18 '12 at 17:21

2 Answers 2

up vote 5 down vote accepted

Well, with this, you can easily login with just the email, here's why :

The default query is :

SELECT aes_decrypt(password, 'asdasdasd') AS password FROM users WHERE email='$email';

Which mean, if :

$email = "bob@gmail.com";
SELECT aes_decrypt(password, 'asdasdasd') AS password FROM users WHERE email='bob@gmail.com';

Now, If I do this instead :

$email = "dontcare' UNION SELECT "mypass" AS password FROM users WHERE email='bob@gmail.com' LIMIT 1, 1";
$pass = "mypass";
SELECT aes_decrypt(password, 'asdasdasd') AS password FROM users WHERE email='dontcare' UNION SELECT "mypass" AS password FROM users WHERE email='bob@gmail.com' LIMIT 1, 1

Your code will receive the second query, get the password that will match "mypass", return true, so will auth me for the bob@gmail.com account.

Now, with "just" the code you showed us, an attacker can't display a whole table, but since the author of the code didn't take care of securing user input, I'd say that if it's not in that piece of code, there is a high probability that it's still possible some place else.

share|improve this answer
    
Ah very well answered. I couldn't work it out myself. I knew there must be a way. Time to get some mysql_real_escape_strings in there now. :) Thanks for the help. –  Thomas Clayson Jun 18 '12 at 13:11
    
Thank you :) Have you tried it, just to see if it works like I wrote it? (It sure is a vulnerability, but maybe what I wrote fails for some reason). –  Cyril N. Jun 18 '12 at 13:18
    
I think entering Email: foo' OR '- and Password: bar' OR '- is also a valid sql injection technique. –  Terry Chia Jun 18 '12 at 13:22
    
Not sure, since it use aes_decrypt and compare the password after, not directly in the sql query. –  Cyril N. Jun 18 '12 at 13:24
    
Two things happen. Firstly the server has magic_quotes on. So it automatically escapes the instances of ' in the code. When I do stripslashes($email) another thing happens. Prior to the query in the question it first runs a query looking for records with the $email in it. E.g SELECT * FROM users WHERE email='$email' and then checks mysql_num_rows($result) to make sure that the email is actually in the database. When there's two queries this throws an error and the code never gets to the bit where the password is checked. Don't know if this is genius design or pure coincidence. :p –  Thomas Clayson Jun 18 '12 at 13:28

cx42net's answer is wrong. The mysql_query() function only processes a single MySQL statement so in order to carry out an SQL injection you'd need to use a UNION.

The real WTF here is that if the query returns no rows and the password is blank then the user is authenticated as whatever user they presented!

share|improve this answer
    
Ah well spotted. :) This is circumvented in the actual code by a prior check for the email address in the table before getting to this point. However, that too can be circumvented by using the email address dontcare' OR 1=1 OR email='dontcare as it is just a simple mysql_num_rows($result) > 0 check. –  Thomas Clayson Jun 18 '12 at 15:45
    
This is a nightmare! haha –  Thomas Clayson Jun 18 '12 at 15:48
2  
@ThomasClayson - Simple solution. Do not use the code until the new code is ready. –  Ramhound Jun 18 '12 at 17:41
    
You are right, I updated my answer. –  Cyril N. Jun 19 '12 at 4:35

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.