[RESOLVED] SQL Injection Prevention

[Derlin]

I'm developing a bit of message board software, and I was wondering some good ways to prevent SQL injections. Right now, any thing in $_GET that ends up in a query is numerical, so I've been looking at 2 functions: intval() and is_numeric().

My question is which would be better and more beneficial to do... Should I run something like this:

if(is_numeric($var)){
  -- do stuff here --
}else{
  die('Hack attempt was detected')
}

Or something simpler like this:

$var = intval($var)

Or should I run them both? What about any other methods?

[matto]

not too elegant but works for me (to be placed on the begin of a script; example):

if(!is_numeric($_POST['somevalue'])) die("Error ...");

or, in more detail:

if(isset($_POST['submitbuttonname']) && !is_numeric($_POST['somevalue'])) die("Error ...");

In order to prevent users from entering non-numeric values into text fields, use (client-side) javascript.
Then, when the form values are processed, you can assume that it's a manipulation if such a wrong value appears. -> Terminate script without too much words.

Also, in order to avoid manipulations, you should use the $POST and $GET method of receiving form values, not the standard register-globals way.

[Plasma]

One handy method to do it is called typecasting (force a variable to have a specific variable type, not a varient/anything).

For example, instead of checking for an integer or using intval, it would be fine to use:

$id = (int) $_GET['id'];

This has a few benefits.
It always ensures $id is an integer, it cannot possibly be anything else.

If $_GET['id'] is undefined/blank, $id will == 0 (since converting '' to an integer is zero). Pretty much the same with any string/non-numeric value provided.

Finally, if its a string value, you need to escape it if its for use in a database.

If magic_quotes_gcp is OFF (check php manual to determine if its on or off), you need to escape stuff manually:

$string = addslashes($string);

Otherwise, you get the variables in escaped form already and you dont need to do anything (unless your wanting to remove the escaping, for example echoing back to the user).

A simple test is a page with:

<?php
$string = $_GET['string'];

echo "Got: $string";

?>

If ...?string=a'b

Comes back as:

Got: a'b

Then magic quotes is OFF (the ' is not escaped).
If it comes back as:

Got: a\'b

Then its escaped (and can just insert into the database as is).

As for query inserting, always quote field values (eg:

$query = "INSERT INTO table (field) VALUES ('$data')";

So $data cannot break out (when escaped) of the field.

Thats the jist of it!

[Derlin]

Plasma,

magic_quotes_gpc is on, and as common practice, I always quote field values in my queries. So what your telling me is that all I should really need to do is use typecasting, and I should be fine against SQL Injection attacks?

Thanks for all your help. 🙂

[Plasma]

Another method to look out for is modifying the sql query, for example:

Say you have a table named 'users', where you retrieve their name and email address, table structure:

users
- name
- password
- email

perhaps you have a query like:

$query = "SELECT name, email FROM $table";

For whatever reason (this is a basic example, perhaps your using a tablename via a form variable for some reason).

$table would normally be 'users'.

Then someone changes it from 'users' to 'users UNION SELECT name, password AS email FROM users'

our query becomes:

SELECT name, email FROM users UNION SELECT name, password AS email FROM users

So now it would return double the results, but now their password instead of an email address (and your script would render the results back to the user).

Its a basic example, but you always need to validate input to query strings.

[Sxooter]

You shoulds always use the escape function built for your database.

For MySQL it's mysql_escape_string

For PostgreSQL, it's pg_escape_string

and so on. If you use those functions, then anything that would be able to be used for injection should be defeated, PLUS you're guaranteed to have it in the proper format for insertion.

[Derlin]

What if I did something along the lines of...

foreach($_GET as $get)(
  $get = mysql_escape_string($get);
}

Since none of the default $_GET vars I'll be using should ever need to be escaped, this would only pose a problem for the pesky would-be crackers. And in the off-chance that i'd ever need to use an escaped character, I could just run it through stripslashes()...

...Right?

[Drakla]

Well you're talking about a specific script if you're saying that none of your GET vals need to be escaped, but I think most people were thinking of the bigger picture.

If you are putting anything into a database then definitely use the appropriate escape function as Sxooter says - even if this means stripslashing everything because magic quotes is on, and then escaping it again.

Once you start coding day after day you'll realise that loads of time is spent tediously checking if your input is escaped or not, I want data I can use straight away so if I'm dealing with input I've created a class that when created takes whatever input you want, GET, POST or REQUEST and immediately strips it if magic quotes is on, you then deal with that.

I'll paste a stripped down version and how I use it. It also has loads of verification built in, but that's not for today, Parappa

class RequestHandler
{
	var $data;				// copy of request data, auto de-slashed if need be

function RequestHandler($data = NULL)
{
	$this->data 		= $data ? $data : $_REQUEST;

	if (get_magic_quotes_gpc())
		$this->strip();
}

function strip()
{
	$this->recurse_it($this->data, 0, 'stripslashes');
}

function trim()
{
	$this->recurse_it($this->data, 0, 'trim');
}

function specialchars()
{
	$this->recurse_it($this->data, 0, 'htmlspecialchars');
}

function escape()
{
	$this->recurse_it($this->data, 0, 'mysql_real_escape_string');
}

	// hope you appreciate how lovely this is

function recurse_it(&$in, $dead, $function)
{
	if ( is_array($in) )
		array_walk($in, array($this, 'recurse_it'), $function);
	else
		$in = $function($in);
}
}



$rh = &new RequestHandler($_POST);

// check for something

if (!isset($rh->data['foo']))
{
	die('I need foo!');
}

// it needs to go to the db now

$rh->escape();

// create a query from the data

$fields = array('name', 'surname', 'address', 'blah', 'whatever', 'minger');

$queryBits = array();
foreach ($fields as $field)
{
	$queryBits = $field . ' =  "'.$rh->data[$field].'"';
}
$q = 'UPDATE mytable SET '.implode(',', $queryBits).' WHERE id = '.$rh->data['id'];

[Derlin]

Okay, I think I have a handle on all of this. But now I have another question... What is the difference between addslashes() and mysql_escape_string()? DOn't they do the same thing, putting a \ infront of ', ", \, etc?

[Sxooter]

SQL injection attack