Securing $_GET[]
Moderator: General Moderators
Securing $_GET[]
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
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
-
peterjwest
- Forum Commoner
- Posts: 63
- Joined: Tue Aug 04, 2009 1:06 pm
Re: Securing $_GET[]
Int:
Alpha numeric:
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(is_int($_GET['subject'])) {
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
else { echo "oh no"; }
}Code: Select all
if(isset($_GET['subject'])){
if(ctype_alnum($_GET['subject'])) {
$aFAQs = $oFAQ->getFAQs($_GET['subject']);
}
else { echo "oh no"; }
}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[]
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!
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[]
is_int() won't work - all input in $_GET and $_POST are strings, regardless of content. Use ctype_digit instead.
Re: Securing $_GET[]
I've been using PHP for a little over 10 years and I never knew about the ctype_ functions until today. 
Re: Securing $_GET[]
Would it not be better to just type cast it as an int?
Like...
Unless you want to display an error message or something...then yeah, you can use the ctype_digit() function.
Like...
Code: Select all
if(isset($_GET['subject'])){
$aFAQs = (int) $oFAQ->getFAQs($_GET['subject']);
}-
peterjwest
- Forum Commoner
- Posts: 63
- Joined: Tue Aug 04, 2009 1:06 pm
Re: Securing $_GET[]
Sorry, I assumed PHP would recognise the string as an int. In that case is_numeric() won't work.
This will match your integer:
Or you could use this: (you want to cast to an int before feeding into the method)
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"; }
}Code: Select all
if(isset($_GET['subject'])){
$aFAQs = $oFAQ->getFAQs((int) $_GET['subject']);
}Re: Securing $_GET[]
Yeah, that's what I meat to putpeterjwest 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']); }
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
Re: Securing $_GET[]
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?
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[]
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
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
-
peterjwest
- Forum Commoner
- Posts: 63
- Joined: Tue Aug 04, 2009 1:06 pm
Re: Securing $_GET[]
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.