[SOLVED] Catch All for GET/POST SQL Injections?

[Stryves]

Hello, I use the following PHP file to connect to the database.

 

I'm curious if by using this, will it take all GET/POST data and automatically run mysql_real_escape_string on all data submitted? I found this from a snippet I downloaded, and was curious if this was better than adding the 'mysql_real_escape_string' on all POST variables.

 

<?php
$db = mysql_connect("*****", "*****", "*****") or die("Dear Sweet Jebus!");
if(!$db)
die("no db");
if(!mysql_select_db("*****",$db))
  die("No database selected.");
if(!get_magic_quotes_gpc())
{
  $_GET = array_map('mysql_real_escape_string', $_GET);
  $_POST = array_map('mysql_real_escape_string', $_POST);
  $_COOKIE = array_map('mysql_real_escape_string', $_COOKIE);
}
else
{
   $_GET = array_map('stripslashes', $_GET);
   $_POST = array_map('stripslashes', $_POST);
   $_COOKIE = array_map('stripslashes', $_COOKIE);
   $_GET = array_map('mysql_real_escape_string', $_GET);
   $_POST = array_map('mysql_real_escape_string', $_POST);
   $_COOKIE = array_map('mysql_real_escape_string', $_COOKIE);
}

?>

[Crew-Portal]

Well yes. Using mysql_real_escape_string(); is VERY important for all POST Variables. if anyone used MySQL syntax's then they could easily alter your database. And since the server is already connected they wouldn't need User or Pass. You may also want some strip slashes in there too and a MD5 encrypt wouldn't hurt either.

[Stryves]

I agree all post variables should be ran through 'mysql_real_escape_string'. Is this above script running the command on all post variables, or should you specify it each time?

 

For example, on my site.

 

<?php
  $username=$_POST['username'];
  $password=$_POST['password'];
?>

 

Would you add the 'mysql_real_escape_string' again to this post, or does having those lines in the DB connect take the post material automatically and check it?

[discomatt]

I... guess... but you're performing a lot of needless operations.

 

I prefer to sanitize only what I need to, when I need to. Doing everything every time is slow, and may cause the contents of your variables to be hard to predict.

[Stryves]

hmm I see. But if all POST data should be checked, why wouldn't you check it?

 

I have a lot of data in forms being past through a select/option list, and is this what you would not check?

[.josh]

You need to check even those.  Form values can easily be altered.

[Stryves]

As always, I thank everyones input!

 

So then what I am doing overall cleans all input, but what is it doing that is needless operations then?

 

I was just thinking if I am cleaning a form, why have the real escape string written everytime if a catchall can do the job?

[discomatt]

There's nothing wrong with it. It's just lazy and inefficient. Any time you want to make a comparison or grab 'expected' results, you have to keep in mind they've been modified.

 

<pre><?php

mysql_connect( 'localhost', 'root', '' );

$var = "You shouldn't use that method";
$var_escaped = mysql_real_escape_string( $var );

echo substr( $var, 4, 9 ) ."\n";
echo substr( $var_escaped, 4, 9 );

?></pre>

 

Output

shouldn't
shouldn\'

 

Any changes made in the future to mysql_real_escape_string() will also affect any hacks you use to accommodate your new modified variables.

 

Escape only what you have to when you have to. It's really not that hard if you're coding smart.

[discomatt]

<pre><?php

## BAD, TEDIOUS ##
$somevar1 = mysql_escape_string( $_POST['somevar1'] );
$somevar2 = mysql_escape_string( $_POST['somevar2'] );
$somevar3 = mysql_escape_string( $_POST['somevar3'] );
$somevar4 = mysql_escape_string( $_POST['somevar4'] );
$somevar5 = mysql_escape_string( $_POST['somevar5'] );

## BETTER, EASY TO DO RIGHT BEFORE QUERY ##
$vars = array(
'somevar1',
'somevar2',
'somevar3',
'somevar4',
'somevar5'
};

sanitize( $vars );

function sanitize( $mixed ) {
if( is_array($mixed) )
	foreach( $mixed as $var )
		$GLOBALS[$var] = mysql_real_escape_string( $_POST[$var] );
else
	$GLOBALS[$mixed] = mysql_real_escape_string( $_POST[$mixed] );
}

?></pre>

 

Just a quick sample. Using implode() you'd even be able to build a sanitized SET string for your query... all without touching the original values of your requested data, and without performing any unnecessary operations or creating any unnecessary variables.

[Stryves]

Thank you so much!

 

Makes a lot more sense to me now.