Web Hosting Talk







View Full Version : Prevent SQL Inject


latheesan
02-13-2006, 12:41 PM
Hello,

I came across this page http://us2.php.net/manual/en/function.mysql-real-escape-string.php (Example 3) about how to prevent sql inject.

So i constructed my own script like this:

<?php

include("../vars.php");

function safe($query)
{
if (get_magic_quotes_gpc())
{
$query = stripslashes($query);
}
if (!is_numeric($query))
{
$query = "'" . mysql_real_escape_string($query) . "'";
}
return $query;
}

mysql_connect($db_host, $db_username, $db_password) or die(mysql_error());
mysql_select_db($db_name);

if($_POST['cmd'] == "Login")
{
$query = sprintf("SELECT * FROM admin_config WHERE username=%s AND password=%s",
safe($_POST['username']),
md5($_POST['password']));

$result = mysql_query($query);

if(mysql_num_rows($result) == 1)
{
echo "Correct Login";
} else {
echo "Wrong Login";
}
}

mysql_close();
?>

<form action="test.php" method="post">
<table border="0" width="100%">
<tr>
<td><input type="text" size="20" name="username"></td>
</tr>
<tr>
<td><input type="text" size="20" name="password"></td>
</tr>
<tr>
<td><input type="submit" name="cmd" value="Login"></td>
</tr>
</table>
</form>

When i execute the test.php script, no errors, but when i enter the correct login details press login button, i get the following message:

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in C:\server\xampp\htdocs\test.php on line 29
Wrong Login

How can I get it to work?

Are there any DB Query Class out there with Anti-SQL Inject feature?

Thanks in advance for your help.

deuce868
02-13-2006, 02:30 PM
Check out ADODB. I actually use ADODB-lite and with it you can setup prepared sql statements that will be escaped nicely.

Dan L
02-13-2006, 02:52 PM
Why are you adding single quotes around the mysql_real_escape_string?

Korvan
02-13-2006, 03:46 PM
you dont need to add those quotes, thats probably causing the error.

you safe function should only handle user input before inserting into an sql statement. You also arent catching your mysql errors

for instance the code exicution level might look like this:

define("_DEBUG", true);
$query = NULL;
$result = NULL;
$data = $_POST['data'];
$data = safe($data);
$query = "INSERT INTO mytbl (junk) VALUES ('{$data}');";
//execute query
if(!( $result = mysql_query($query)) )
{
//error
if(_DEBUG)
echo(mysql_error());

//log the error
}
else
{
//run all the fun stuff
}


taking a look at your code, the problem is you arnt single quoting your md5 password string.
in safe()

$query = "'" . mysql_real_escape_string($query) . "'";

should be

$query = mysql_real_escape_string($query);


and in your query line

$query = sprintf("SELECT * FROM admin_config WHERE username=%s AND password=%s",
safe($_POST['username']),
md5($_POST['password']));


should be


$user = safe($_POST['username']);
$pass = md5($_POST['password']);
$query = "SELECT * FROM admin_config WHERE username='{$user}' AND password='{$pass}';";


I usually do the single quoting at the query level after checking the type of the variable. This avoids confusion.
If the variable is of the wrong type, you need to throw an error.

If you want to use your current function its fine, just remember to put single quotes around any string you do not pass to that function

So your alternate solution to keep your safe function the same would be to do this:

$query = sprintf("SELECT * FROM admin_config WHERE username=%s AND password=%s",
safe($_POST['username']),
md5($_POST['password']));


to

$query = sprintf("SELECT * FROM admin_config WHERE username=%s AND password=%s",
safe($_POST['username']),
"'" . md5($_POST['password']) . "'");

latheesan
02-13-2006, 04:16 PM
Wow man, thanks alot for your help Korvan.

Got it to work just like how i want it :D