Page 1 of 1
Securing $_GET[]
Posted: Wed Sep 16, 2009 9:59 am
by borami
Hi All,
This is my 1st PHP post! I am an ASP programmer by trade, but I have this issue with a legacy application in PHP. The clietn is worried about the security of the code on the site as all $_GET requests are accepting anything.
For example:
if(isset($_GET['subject'])){
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
I want to make sure that the value 'subject' is an integer. In PHP i can check this with (int) - I just don't know enough on the syntax to understand where in the above code it would go?
I also want to check that some $_GET are just alphanumeric too - what would I use for that and where?
Any advice would be appreciated.
Thanks
Alex
Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:13 am
by peterjwest
Int:
Code: Select all
if(isset($_GET['subject'])){
if(is_int($_GET['subject'])) {
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
else { echo "oh no"; }
}
Alpha numeric:
Code: Select all
if(isset($_GET['subject'])){
if(ctype_alnum($_GET['subject'])) {
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
else { echo "oh no"; }
}
Although you'll probably want a custom set of characters in which case you'll want to use a regex, such as:
Code: Select all
if(isset($_GET['subject'])){
if(preg_match('~^[a-zA-Z0-9 _]+$~'$_GET['subject'])) {
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
else { echo "oh no"; }
}
Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:21 am
by borami
I thinks im a convert to PHP - at least you guys actively read forums and contriobute - more than can be said for the ASP communities
I will try that out to see.
Also On the same subject - I am also using mysql_real_escape_string($SubjectID) in each db query, is this neccessary or just overkill if I already check the values im expecting to be passed?
Thanks Peter!
Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:32 am
by requinix
is_int() won't work - all input in $_GET and $_POST are strings, regardless of content. Use
ctype_digit instead.
Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:37 am
by onion2k
I've been using PHP for a little over 10 years and I never knew about the ctype_ functions until today.

Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:47 am
by jackpf
Would it not be better to just type cast it as an int?
Like...
Code: Select all
if(isset($_GET['subject'])){
$aFAQs = (int) $oFAQ->getFAQs($_GET['subject']);
}
Unless you want to display an error message or something...then yeah, you can use the ctype_digit() function.
Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:57 am
by peterjwest
Sorry, I assumed PHP would recognise the string as an int. In that case is_numeric() won't work.
This will match your integer:
Code: Select all
if(isset($_GET['subject'])){
if(preg_match('~^[0-9]+$~'$_GET['subject'])) {
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
else { echo "oh no"; }
}
Or you could use this: (you want to cast to an int before feeding into the method)
Code: Select all
if(isset($_GET['subject'])){
$aFAQs = $oFAQ->getFAQs((int) $_GET['subject']);
}
Re: Securing $_GET[]
Posted: Wed Sep 16, 2009 10:58 am
by jackpf
peterjwest wrote:Or you could use, you want to cast to an int before feeding to the method.
Code: Select all
if(isset($_GET['subject'])){
$aFAQs = $oFAQ->getFAQs((int) $_GET['subject']);
}
Yeah, that's what I meat to put
Of course you'll want to cast it as an int before you pass it to the function...my mistake.
Or maybe even in the function itself

who knows.....
Re: Securing $_GET[]
Posted: Thu Sep 17, 2009 3:12 am
by borami
Wow - thanks for all the feedback. I implemented the ctype approach (I realised through trial and error that is_int would not work).
So now all $_GET requests are checked for data type before being passed to SQL statements. Once passed they are run through 'mysql escape special characters'.
So hopefully this should prevent most SQL injection attacks ? Unless anyone can suggest any big flaws in what i've done so far?
Re: Securing $_GET[]
Posted: Thu Sep 17, 2009 3:17 am
by jackpf
You don't reeeally need to pass integers through mysql_real_escape_string() (or whatever you're using), since because it's only an integer, it's not going to contain any odd characters that can be used to hack you (I believe

)
But I guess it can't hurt.
As long as you run all user supplied data through an SQL escape function before you use it in a query, you should be fine...
That's for SQL injection anyway. There are plenty of other things you need to look out for, like XSS and stuff...
Just like you should run all user supplied data through an escape function before you put it in a query, you should also run any user supplied data through a HTML encoding function before you display it

Re: Securing $_GET[]
Posted: Thu Sep 17, 2009 7:31 pm
by peterjwest
If you can automate it so all user data is passed through mysql_real_escape_string() before being input to the database then you will be better off. If you decide later on that your number should be a string instead, you won't have to worry about it.