...

View Full Version : $_GET data type validation



cfructose
03-27-2010, 04:45 PM
I want to decide which data to display by checking a URL query string for an id.

The links are of the form:


echo "<a href='".$_SERVER['SCRIPT_NAME']."?id=".$i."'>BLAH</a>";

where $i is just an incrementing integer in a for loop.

I'm wondering, for security reasons, whether it's necessary to validate the data type before doing stuff with it. Is there any potential harm a hacker could do by typing some script into the URL in place of the value of 'id' that I'm expecting to be a number?

So, esentially, what I'm asking is: is it overkill to validate that the $_GET data is numeric, like so...?


if (isset($_GET['id']) && is_numeric($id)) {
//echo the data in some array where key == $id
}

I guess that the answer is that if I'm only planning on echoing something in an array, then the worst that could happen is that if the 'id' of the query string isn't numeric, then nothing will be printed, and I'll get an error, but nothing worse could happen.

Am I underestimating the risks?
Is such validation good practice regardless?
What do you guys do?

Cheers.

MattF
03-27-2010, 04:53 PM
Is such validation good practice regardless?

Always.

cfructose
03-27-2010, 05:01 PM
Short but sweet!

OK, in that case, I suppose I should also check that the number falls within the range I'm expecting...

Given that the 'id' of the query string will always have a value that's <= the size of my array, I could do:


$validated = (isset($_GET['id']) && is_numeric($_GET['id']) && $_GET['id'] <= count($arr)) ? TRUE : FALSE;

...and then proceed if $validated == TRUE.

Is this really standard practice? Wow!

I'm curious though, what would the potential dangers be? I'm not sure I see any. (While I'm more than happy to take your advice and code this way "ALWAYS", I'd like to have a better understanding of what the risks are if I don't).

MattF
03-27-2010, 05:09 PM
Two simple examples which spring to mind.

1) If $id was used in a DB query and was unsanitised and unescaped, you have potential for a SQL injection.

2) If $id was a filepath for an include, an unsanitised/unvalidated var would have the possibility for a directory traversal vulnerability.


Checking against an array key, as in your case, is fairly harmless. It will either match or it won't, but you should still validate it as being an integer at minimum. For correctness if nothing else. The primary reason for draconian validation and sanitisation at all times is to make sure that you don't forget at some point. If you're always draconian with your input, the chance of introducing a vulnerability/exploit accidentally at some point in the future becomes reduced drastically.

cfructose
03-27-2010, 08:28 PM
SQL injection attacks if the var is for a database is a risk I'm well acquainted with. The filepath idea, however, is one that I wouldn't have thought of. I appreciate the examples.


the primary reason for draconian validation and sanitisation at all times is to make sure that you don't forget at some point.

Nicely put. Henceforth, my middle name shall be 'Draconian'.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum