question mysql security.... is this function practical, safe enough?

[mac007]

Hello, all: I'm a newbie and I'm trying to understand the whole php security thing a bit better, and found this function that seems easy to implement, as well as easy to understand... my question is...

 

Does this means I could use regular $_POST, $_GET, $_COOKIE, $_REQUEST variables without having to individually worry about escaping them every time I use them in queries... is this correct? or safe enough? see example below, along with how I'm using variable in a query...  it seems to be working fine, as it does echo out the \ escape character when I test it, or when I try to inject it...

 

Appreciate your input!

 

 

<?php

$_POST=sanitize($_POST);
$_GET=sanitize($_GET);
$_COOKIE=sanitize($_COOKIE);
$_REQUEST=sanitize($_REQUEST);

function sanitize($input){
if(is_array($input)){
foreach($input as $k=>$i){
$output[$k]=sanitize($i);
}
}
else{
if(get_magic_quotes_gpc()){
$input=stripslashes($input);
}
$output=mysql_real_escape_string($input);
}
return $output;
}


// mysql query then I could use be:

$money = $_GET['money'];
$result = mysql_query("SELECT * FROM countries WHERE currencies = '$money'");


?> 

[jskywalker]

no, because if someone posts something like:

$money="'; delete form countries"

 

than it would delete the contents from your countries table.......

[mac007]

Thank you jsky...

 

well, I think the "sanitize" function is properly escaping the single/double quotes for every $_POST, $_GET variables as they get input... seems to be working OK as pere my testing so far...  am I wrong in assuming that, and they are not being sanitized properly??

[PFMaBiSmAd]

mysql_query() does NOT support multiple queries.

[mac007]

Thank you PFM... is that a comment on my code, or a comment referring to jsky's??  not sure what you mean by "NOT support multiple queries"...

[PFMaBiSmAd]

The code you posted does protect against sql injection in STRING data (i.e. data you put between single-quotes in a query.) Because it uses mysql_real_escape_string() on the data.

 

It however does not protect against sql injection in numerical data (i.e. data that is not put between single-quotes in a query.) For numerical data, you must validate that it is numerical or simply cast it as a number before you put it into a query. The reason for this is that it is possible to craft a query that does not use any quotes in it that injects a UNION query to dump all the data in your table. When this type of injection is used in STRING data, it is just treated as data. When this type of injection is used in numerical data it becomes part of the query.

 

 

[PFMaBiSmAd]

My post about multiple queries referred to jskywalker's post.

[mac007]

got it... thanks a lot for the explanation. So the code I have in general is in right direction, EXCEPT that I should first validate/cast variable for a numerical value, and then if it's NOT numerical,  pass it thru final mysql_real_escape_string function as final check???  (there would be no need to pass thru mysql_real_escape_string function if it's a numerical)

[jskywalker]

and you should NOT do  $money = $_GET['money'];  just before your query....... 

 

@PFMaBiSmAd: you are right about the multiple query's...

[mac007]

thanks jsky... is it because if I do $money = $_GET['money']; then it woudl bypass the "sanitize" function?? I'm assuming that by declaring it $_GET['money'], then it would automatically be picking up the function as it sanitizes anything that gets posted thru $_GET or $_POST... when I test it, seems to be doing it right, unless I'm testing it wrong...

 

Thanks...

[jskywalker]

damn, i need coffee, thats for sure.....

 

 

[mac007]

me too!

[PFMaBiSmAd]

Using $money = $_GET['money'] would not bypass your sanitize function, because the $_GET variables are already escaped by your function. It would however add an unnecessary line of code and the memory needed for the $money variable. You can just use $_GET['money'] anywhere you need to use it.

 

If you are going to reference a variable more than one time in your code, you can save a little typing by creating another (shorter named) variable from a variable like $_GET['money'].

 

Back to your original post, don't use $_REQUEST, ever. Because it combines get, post, and cookie, you end up overwriting values if you forget you already have used a same name variable in your application and it makes it a little easier for hackers to feed your code the hackers values for post and cookie data by simply trying things on the end of the URL that is used to request your page.

 

 

[mac007]

Thanks again PFM... I wasnt aware of that REQUEST issue! wow. Also, yes, I was using the $money variable as a way to shorten it, and easier to remember it in case I needed to use it multiple times. So, I modified the function to account for numeric-validation... woudl this work? (i inserted it right after check for magic-quotes, but just before striplashes function)

 

$_POST=sanitize($_POST);
$_GET=sanitize($_GET);
$_COOKIE=sanitize($_COOKIE);

function sanitize($input){
if(is_array($input)){
foreach($input as $k=>$i){
$output[$k]=sanitize($i);
}
}
else{
if(get_magic_quotes_gpc()){
if (is_numeric($input)) {
$output = $input;
}
else {
$input=stripslashes($input);
}
$output=mysql_real_escape_string($input);
}
return $output;
}

[/code>

[mac007]

well, I tested it, and I think it's more like this... seems to be working I believe (checking if is NOT numeric)

 

 

function sanitize($input){
if(is_array($input)){
foreach($input as $k=>$i){
$output[$k]=sanitize($i);
}
}
else {
if(get_magic_quotes_gpc()){
if (!is_numeric($input)) {
$input=stripslashes($input);
}
}
$output=mysql_real_escape_string($input);
}
return $output;
}