SQL Injection Vulnerability found

Question

Hello All,

Yesterday i received an email from a guy that our site is vulnerable to SQL injection. The email said:

I tried some classic SQL injection on your server. This URL contains the result:

http://www.mysite.com/ppreview.php?id=611111161%20and%201=0%20UNION%20all%20SELECT%201,2,3,4,password,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,user%5Fid,70,71%20%20from%20admin--&u=10064&t=users%5Fcars

Note that in the above URL, i do not expose my actual domain and have replaced it with mysite.com.

Can any one explain what above URL means as my site is vulnerable to that sort of url and possibly to your sites too.

How to decode that url, what is happening there?

Answer 1

The problem is that you're concatenating that SQL command on the query string into your SQL command.

Presumably your code says something like

"select * from preview where ID=" + Request.QueryString["id"]

Once you use that QueryString it becomes

select * from preview where ID=611111161 and 1=0
UNION ALL
SELECT 1,2,3,4,password,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,user=id,70,71
FROM admin

EG: He's made your admin account logins show up on your preview page.

You should always make sure to SQL escape any inputs you get from the user, or even better use parametrized queries and the server will take care of that. Without knowing the language or the type of SQL server I can't really point you in the direction of what code you'd need to do that.

Answer 2

You probably have code that looks like (I don't know php syntax):

string sql = "select * from mytable where customerid = " + Request.QueryString("id");

Now, since the guy who mailed you added a lot more than just the id to your page's querystring, your sql statement is going to like like:

select * from mytable where customerid = 6111111661 and union all the tables that you don't want.

Always use parameters in your queries and check the user input! Try to avoid dynamic sql if possible.

Answer 3

Change your code to work like this.

$id = $_GET['id'];
$user = mysql_real_escape_string($_GET['user']);

if( is_numeric($id) )
{
   mysql_query($query);
}

Now your code won't accept an invalid user ID and you won't have issues with SQL injection so long as you sanitize all strings using the method I've outlined.