SQL Injection attack

[williamZanelli]

Hi guys,

 

One of my friends pointed out kindly that my website was vulnerable to SQL injection attacks as I was using

 

$pageid = $_GET ["id"]

 

He showed by adding the following line to my URL he could access some extra info

 

php?id=117+union+select+1,database(),user(),1,version(),1,1+--

 

I was advised to change this to

 

$pageid = mysql_real_escape_string($_GET ["id"]); 
$sql_1 = "Select * from messages WHERE id = $pageid ";

 

 

Now the problem is even though I've "real escaped" the URL, If I was to add the follwing line to my URL

 

php?id=117+union+select+1,database(),user(),1,version(),1,1+--

 

I still get the DB version, logcat etc..

 

How do I secure my website?

 

Thanks in advance for your thoughts in advance

 

Will

[KevinM1]

You should know what page id's are legit.  Simply test the incoming id against the allowable ones.  If they don't match, deny access.

[aggrav8d]

does your query say

 

WHERE pageid=$pageid

 

or does it say

 

WHERE pageid='$pageid'

 

?  The second is better.

[PFMaBiSmAd]

mysql_real_escape_string(), like its name states is only useful for escaping string data. You are using it on data in a query that is expected to be numeric. You need to validate that the data is numeric (or cast it as an integer) and that it has an expected range of values, like Nightslyr has suggested. Ignoring the sql injection aspect, what does your existing code do when you execute a query for a page that does not exist?

[williamZanelli]

My SQL is as follow

$sql_1 = "Select * from messages WHERE id = $pageid ";

 

I'm not using the quote marks as the $pageid is a nuumber field in the DB.

 

I've made no extra provisions for a page that does not exist. ie. if $pageid doesnt't exist it returns nothing. How should I deal with this?

 

@PFMaBiSmAd can you provide me with an example of how other people deal with this?

 

Thanks for your thoughts guys

 

Will

[PFMaBiSmAd]

If your existing code correctly handles a query that does not match any rows, then casting the value as an integer would be enough -

 

$pageid = (int)$_GET["id"];

[williamZanelli]

Thanks for that PFMaBiSmAd.

 

Now just to get my head around this one... say if $pageid wasn';t an integer, how would I handle this then?

 

Thanks

 

Will

[Daniel0]

Delimit it in quotes. IDs should be integers though.

[nadeemshafi9]

Delimit it in quotes. IDs should be integers though.

 

how good of a solution is that because i am having issues wit hthis too, prior to my new apps i used to have an array in the bootstrap of the apps and used to check against it for illagel chars for id's and text strings in teh url or post directly in teh model on any controler call. That caght everything.

 

now i am using the zend MVC and ajax its slightly more of a widespread issue zend seem to havre made it slightly better because they break up the query into functions eg select() from() etc.

 

how well can i protect myself by using quotes ? can it be broken still ? does quoting it fix any vulnrability ?

[Daniel0]

If you're using ZF's Zend_Db, you can just call the find() method on the Zend_Db_Table descendant to search by the PK you've set. That will be fine.

 

Or maybe like:

$something = $somethingTable->select()->where('id = ?', $id);

[nadeemshafi9]

If you're using ZF's Zend_Db, you can just call the find() method on the Zend_Db_Table descendant to search by the PK you've set. That will be fine.

 

Or maybe like:

$something = $somethingTable->select()->where('id = ?', $id);

 

and that will filter any attacks from being injected because if teh firts part fails it breaks ?

[Daniel0]

It's using prepared statements, so it will be safe, yes. Unless you have a row with the a PK like '; DROP TABLE users; -- it will just return 0 rows.

[nadeemshafi9]

It's using prepared statements, so it will be safe, yes. Unless you have a row with the a PK like '; DROP TABLE users; -- it will just return 0 rows.

 

ok cool cos my supervisor wasent worried about sql injection and i was like oh but look you can just look in firebug and see all teh ajax calls and attack them all then i tried it and i manage to break one or two but didnt manage to delete anything.

 

im juts reading up on prepaired statements.

 

can you elaborate a tincey wincey bit more on that example you gave

 

PK like '; DROP TABLE users; -- it will just return 0 rows.

 

plz thanks

[PFMaBiSmAd]

@williamZanelli

Now just to get my head around this one... say if $pageid wasn';t an integer, how would I handle this then?

 

$pageid = (int)$_GET["id"];
var_dump($pageid);
echo "<br />";
var_dump(is_numeric($_GET['id'])); // note, all post/get data are strings, so you cannot use is_int()

 

Results:

yourfile.php?id=123

int(123)

bool(true)

 

yourfile.php?id=117+union+select+1,database(),user(),1,version(),1,1+--

int(117)

bool(false)

 

yourfile.php?id=some string

int(0)

bool(false)

 

Edit: Don't forget that all external data needs to be validated before using it. Relying on something like prepared statements will protect your database against sql injection, but that won't stop a hacker from feeding your code all kinds of nonsense data that would tie up your database server and consume resources. Only execute queries that contain expected user supplied values in them.

[nadeemshafi9]

why not make your get variable into a object with your own syntax and then hash or encode it and then decode it and read it in on the other end.

 

to teh original post, i used to keep track of attacks on my sites and they used to be bots that attack you especialy in the url especialy if you include files using get vars

[Daniel0]

PK like '; DROP TABLE users; -- it will just return 0 rows.

 

Say that $usersTable is an instance of Zend_Db_Table_Abstract. $username is a raw value from the user.

 

You do

$user = $usersTable->fetch($usersTable->select()->where('username = ?', $username)->limit(1));

 

And this will fetch the user with that username. If $username has the value Daniel, that user will be returned, assuming it exists in the database. You can enter any value into $username. If you set $username to '; DROP TABLE users; --, it will search for user's with that username. Assuming no user is called like that, it just won't return any rows the same way that a potentially non-existent Daniel would. Had you used mysql_query() and passed $username straight to it without any precautions you would have lost your users table, but using prepared statements, the attack has been foiled.

[nadeemshafi9]

ok we have stopped the db user from dropping anything anyways but yes.

 

so your saying that quoting into will fix the issue, i use quoteinto for update and delete queries but

 

$result 	= $tvCustomerData_table->fetchAll($tvCustomerData_table->select()
    											->from($tvCustomerData_table)
    											->where("{$search_filter} {$user_filter}")
    											->order(array("{$sort} {$dir}"))
											->limit($limit, $start));

 

is teh above code prone to injection ?

[Daniel0]

It depends on where the variables come from. It's by far easiest to just parametrize the queries as shown before.