hi,

okay i have (rubbish) game and for some reason this hack is still working even after i added mysql_real_escape_string() which claims to stop all SQL injection yet its still working...

PHP Code:

$_GET['ID'] = mysql_real_escape_string($_GET['ID']);
$db->query("UPDATE users SET crystals=crystals+{$r['cmQTY']} where userid=$userid");
$db->query("DELETE FROM crystalmarket WHERE cmID={$_GET['ID']}");
print "Crystals removed from market!<br />
<a href='cmarket.php'>&gt; Back</a>"; 


and the hack url is:
http://forbiddensoldiers.dansgalaxy....999,-999999999

and it also returns this error yet still adds 999999999 to the suers crystals

QUERY ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'union all select 1000,999999999,999999999,-999999999' at line 1
Query was DELETE FROM crystalmarket WHERE cmID=-1 union all select 1000,999999999,999999999,-999999999


anyideaS?!

Try using the serialize() function also, and maybe a few if functions. I also reccomend putting what you get from the URL in a variable. For example

PHP Code:

<?php
$getid = $_GET['ID'];
if(is_numeric($getid)){
// Do the SQL.
} else {
echo "error";
}
?>

For more information go to:
http://uk.php.net/manual/en/function.serialize.php
http://uk.php.net/manual/en/function.is-numeric.php
http://uk.php.net/manual/en/function...ape-string.php

Is_numeric will work in this situation, but it may not be the optimal solution. Consider an id field INT(10). You wouldn't want to allow any numbers greater than 10^9. This can be check like so:

PHP Code:

if(preg_match('/^[0-9]{1,10}$/', $_GET['ID'])) {
// SQL
} else {
echo "Invalid ID";
} 


You need to be validating and casting variables where possible. Try using the sprintf function http://uk3.php.net/sprintf or such functions as intval http://uk2.php.net/intval to ensure $_GET['ID'] is always an integer.

You dont say but if $_GET['ID'] is always expected to be a number then you can ensure its cast correctly by using.

PHP Code:

$id= intval($_GET['ID']);// if ID is a string then $id will be set to 0 


also

PHP Code:

 $id=$_GET['ID'];
$query= sprintf("DELETE FROM crystalmarket WHERE cmID=%d ", $id);// using %d ensures $id is a number
$db->query($query); 


Various other tehniques

even after i added mysql_real_escape_string() which claims to stop all SQL injection yet its still working...

Well see mysql_real_escape_string doesn't claim that at all and anyone that claims that it does doesn't really understand.

Look at http://php.net/mysql_real_escape_string at show us where that claim is made. Look at the URL again and it even tells you the characters that are escaped and you will see that the commas and so on you are trying to escape are not escaped by that function.

The problem is that you are using functions and assuming their outcome, then blaming the functions when they don't work the way you assume them to...

Originally Posted by TwistMyArm

Well see mysql_real_escape_string doesn't claim that at all and anyone that claims that it does doesn't really understand.

Look at http://php.net/mysql_real_escape_string at show us where that claim is made. Look at the URL again and it even tells you the characters that are escaped and you will see that the commas and so on you are trying to escape are not escaped by that function.

The problem is that you are using functions and assuming their outcome, then blaming the functions when they don't work the way you assume them to...

I didnt realise its just everysite seems to say that this is supposed to the THE function to stop SQL injection dead...

I think the problem lies in that you don't have the original value in the query quoted.

$db->query("DELETE FROM crystalmarket WHERE cmID='{$_GET['ID']}');

If you want the value to be numeric, I would skip the string escape and use:

$db->query("DELETE FROM crystalmarket WHERE cmID=" . (int)$_GET['ID'] . ");

or

$db->query("DELETE FROM crystalmarket WHERE cmID=" . intval($_GET['ID']) . ");

Whilst it is important to check that data is valid to prevent SQL injection it should be noted that some of the methods shown here aren't really adequate.

rogem002 and phpknowhow have got the right idea. They suggest checking the value before executing the SQL. The other posts all use casting or a similar method which still results in a SQL query being executed. You really don't want this when someone is delibrately injecting code into your script especially when the SQL is likely to fail and potentially dump an error to the browser depending on the PHP setup.

Always always validate the data BEFORE you use it in any processing. If it fails validation, output your own error message rather than an information loaded PHP error message.