Securing $_GET[]

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
borami
Forum Newbie
Posts: 3
Joined: Wed Sep 16, 2009 9:56 am

Securing $_GET[]

Post 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
peterjwest
Forum Commoner
Posts: 63
Joined: Tue Aug 04, 2009 1:06 pm

Re: Securing $_GET[]

Post 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"; } 
}
borami
Forum Newbie
Posts: 3
Joined: Wed Sep 16, 2009 9:56 am

Re: Securing $_GET[]

Post 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!
User avatar
requinix
Spammer :|
Posts: 6617
Joined: Wed Oct 15, 2008 2:35 am
Location: WA, USA

Re: Securing $_GET[]

Post by requinix »

is_int() won't work - all input in $_GET and $_POST are strings, regardless of content. Use ctype_digit instead.
User avatar
onion2k
Jedi Mod
Posts: 5263
Joined: Tue Dec 21, 2004 5:03 pm
Location: usrlab.com

Re: Securing $_GET[]

Post by onion2k »

I've been using PHP for a little over 10 years and I never knew about the ctype_ functions until today. 8O
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: Securing $_GET[]

Post 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.
peterjwest
Forum Commoner
Posts: 63
Joined: Tue Aug 04, 2009 1:06 pm

Re: Securing $_GET[]

Post 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']);
}
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: Securing $_GET[]

Post 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 :P

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 8O who knows.....
borami
Forum Newbie
Posts: 3
Joined: Wed Sep 16, 2009 9:56 am

Re: Securing $_GET[]

Post 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?
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: Securing $_GET[]

Post 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 :)
peterjwest
Forum Commoner
Posts: 63
Joined: Tue Aug 04, 2009 1:06 pm

Re: Securing $_GET[]

Post 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.
Post Reply