better security for $_GET variables

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
gth759k
Forum Commoner
Posts: 76
Joined: Mon Jun 15, 2009 3:04 am

better security for $_GET variables

Post by gth759k »

Hi! I have a simple page that uses a $_GET variable to retrieve information from my database and can access files on my server with urls like:
script.php?id=4e5b4b1286ccf

the id variable will always be generated with $id = uniqid();

my question is in general ... how do I ensure that only uniqid type id's are passed to my script and ALL OTHER JUNK following "id=" is strictly forbidden from harming my data! Any help would be appreciated. Thanks!
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: better security for $_GET variables

Post by social_experiment »

gth759k wrote:my question is in general ... how do I ensure that only uniqid type id's are passed to my script and ALL OTHER JUNK following "id=" is strictly forbidden from harming my data! Any help would be appreciated.
There's no way to ensure that only your uniqid will be passed along because you don't have control over the user and what they wish to enter into the browser.

However, you can (and should) always assume that whatever is passed to your script via $_GET (or $_POST, $_SESSION, $_REQUEST) has been tainted and should be checked and cleaned.

1. A good start is to check data. If your uniqid is 456, then check in the database that 456 is there before moving on an processing the query. Your a probably already doing this i would say.
2. Use mysql_real_escape_string() to sanitize any input passed along to the database.
3. You can also check the length of your uniqid and if it's longer expected, you know something is up. (This is dependant on a constant length for your uniqid).
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
gth759k
Forum Commoner
Posts: 76
Joined: Mon Jun 15, 2009 3:04 am

Re: better security for $_GET variables

Post by gth759k »

Here is what I have so far. Do you think this is safe enough?

Code: Select all

<?php
	if (isset($_GET['id'])) {
		
		// security first
		$id = strip_tags(addslashes($_GET['id']));
		$id = ereg_replace("[^A-Za-z0-9]", "", $id);
		$id = mysql_real_escape_string($id);
		
		if (strlen($id) == 13) {
			// check that id is in database
			require('config.php');
	
	   	  	$connection = mysql_connect(SQL_HOST, SQL_USER, SQL_PASS) 
                                                 or die('Could not connect to MySQL database. ' . mysql_error());
			$db = mysql_select_db(SQL_DB,$connection);
			$sql = "SELECT id, ext FROM usage_log WHERE id='$id'";
			$result = mysql_query($sql) or die(mysql_error());
			$data = mysql_fetch_array($result, MYSQL_ASSOC);
			
		}
		else {
			// go away!
		}
	}
	else {
		// go away!
	}
?>
Where it says "go away!" what would be the best method to make that happen? Right now I'm just using header ....
User avatar
amirbwb
Forum Commoner
Posts: 89
Joined: Sat Oct 30, 2010 6:10 pm

Re: better security for $_GET variables

Post by amirbwb »

this is very secure,here is anothere way to make it secure if the id contain only numbers you can use is_numeric() function so only numbers are used, but make sure that the id is positive and not negative so here is your code:

Code: Select all

if(is_numeric($_GET['id']) && $_GET['id'] > 0){
$id=mysql_real_escape_string($id);

mysql_select_db();
....

}else{
header('Location:login.php');
//example
}
Last edited by amirbwb on Mon Aug 29, 2011 2:21 pm, edited 1 time in total.
User avatar
phazorRise
Forum Contributor
Posts: 134
Joined: Mon Dec 27, 2010 7:58 am

Re: better security for $_GET variables

Post by phazorRise »

ereg_replace is deprecated. I suggest you to use - php filters and it's types are filter types . Escaping string and adding slashes is fine.
To suppress mysql errors use ' @ ' before mysql functions. eg @mysql_connect.
// check that id is in database
where's the code ?
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: better security for $_GET variables

Post by social_experiment »

Code: Select all

<?php
$id = strip_tags(addslashes($_GET['id']));
$id = ereg_replace("[^A-Za-z0-9]", "", $id);
$id = mysql_real_escape_string($id);
?>
Instead of the first 2 lines, have a regular expression that checks for any character that's not alphanumeric

Code: Select all

<?php
 $trimmed_id = trim($_GET['id']);
 # i think \W will be correct here
 $pattern = '/\W/';
 $match = preg_match($pattern, $trimmed_id);
 // if no match is found, $_GET['id'] contains only alphanumeric
 // characters
 if ($match == 0)
 {
   if (strlen($trimmed_id) == 13)
   {
     $escaped_id = mysql_real_escape_string($trimmed_id);
    // start checking if the id is in the database
   }
   else
   {
      // id incorrect length, notify admin, email an error report, 
      echo 'An error has occured while processing your query';
   }
 }
 else
 {
   // a match against preg_match means a non-alphanumeric
   // character is present.
   echo 'An error has occured while processing your query';
 }
?>
gth759k wrote:what would be the best method to make that happen?
A possibility is to log when a problem arises and display a message about the error. As the user can already see the id value, there is little point in disguising the fact that an invalid id value was found so an error message notifies the normal-i-don't-mess-with-the-querystring user about the problem or they might leave the site wondering why it's not responding / working. Maybe a link to another page or even a contact form.

Code: Select all

<?php
 $sql = "SELECT id, ext FROM usage_log WHERE id='$id'";
?>
Check if the id value exists within the database before going ahead with the query.

Code: Select all

<?php
 $count = "SELECT COUNT(id) FROM usage_log WHERE id = '" . $escaped_id . "' ";
 $countSql = mysql_query($count);
 $countAry = mysql_fetch_array($countSql);
 $rows = $countAry[0];

 if ($rows == 1)
 {
   // id is in the database so you can continue knowing that the
   // id value is in the database.
 }
 else
 {
   // id is not in the database, send error email, link to another
   // page, etc.
   echo 'An error has occured processing your query';
 }
?>
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: better security for $_GET variables

Post by social_experiment »

amirbwb wrote:this is very secure,here is anothere way to make it secure if the id contain only numbers you can use is_numeric() function so only numbers are used, but make sure that the id is positive and not negative so
is_numeric() won't work. The format of the id's are alphanumeric
gth759k wrote: I have a simple page that uses a $_GET variable to retrieve information from my database and can access files on my server with urls like:
script.php?id=4e5b4b1286ccf
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
gth759k
Forum Commoner
Posts: 76
Joined: Mon Jun 15, 2009 3:04 am

Re: better security for $_GET variables

Post by gth759k »

where's the code ?
I forgot it.

Code: Select all

	// check that link is in database
	require('config.php');

	$connection = @mysql_connect(SQL_HOST, SQL_USER, SQL_PASS) 
	  	          	or die('Could not connect to MySQL database. ' . mysql_error());
	$db = @mysql_select_db(SQL_DB,$connection);
	$sql = "SELECT id, ext FROM usage_log WHERE id='$id'";
	$result = @mysql_query($sql) or die(@mysql_error());
	$rows = @mysql_num_rows($result);
	
	if ($rows == 0) {
		// go away
	}
	else {
		// id exists in database
                $data = @mysql_fetch_array($result, MYSQL_ASSOC);
	}
I looked up the php filters, but which one of them filters everything except numbers and characters? The one like FILTER_SANITIZE_URL still leaves special characters.
User avatar
phazorRise
Forum Contributor
Posts: 134
Joined: Mon Dec 27, 2010 7:58 am

Re: better security for $_GET variables

Post by phazorRise »

Code: Select all

echo filter_input(INPUT_GET, 'id', FILTER_SANITIZE_SPECIAL_CHARS);
this will remove special chars from id.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: better security for $_GET variables

Post by John Cartwright »

On the topic is securing the URL, yes this is possible to a degree using hashing.

You basically create a hash of the request parameters you want to enforce are only being used (using a salt and pepper), then when you receive the request renegerate the hash and compare it to the one they provided, otherwise reject the request.

I.e.,

Code: Select all

function createHash($params)
{
   return md5(serialize($params));
}

public function compareHash($params, $hash)
{
   return createHash($params) != $hash;
}

//generating the hash
$request = array('foo' => 'bar');
$hashRequest = createHash($request);
$request['hash'] = $hashRequest;

$url = 'http://domain.com/'. http_build_query($request);

//checking the hash
if (checkHash($_GET, isset($_GET['hash']) ? $_GET['hash'] : null)) {
   //url validated
}
Note the hashing used is not secure, and still requires salt + pepper. This was only used for example purposes.
Post Reply