Receiving information securely from forms

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

dunc
Forum Newbie
Posts: 13
Joined: Mon Sep 04, 2006 10:17 am

Receiving information securely from forms

Post by dunc »

Howdy, me again :oops:

I'm making a list of things I need to do to keep my site secure, and I believe the biggest security risk is the receiving of data from forms.
  • Turn 'register global variables off'
    Not sure whether to use post or get methods?
    Check the data contains no hashes or semi colons
    Make sure the variable contains only letters or only numbers, depending on variable type
What else would you suggest?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Re: Receiving information securely from forms

Post by feyd »

dunc wrote:Turn 'register global variables off'
There's no real need to turn them off as long as you write the script as if they were off and always initialize variables before use (this especially includes arrays.)
dunc wrote:Not sure whether to use post or get methods?
Both are used quite often. Typically, use submissions will be posts while many pages themselves will use gets i.e. ?some=thing
dunc wrote:Check the data contains no hashes or semi colons
Hashes or semicolons? The filtering needed varies a bit depending on the destination, however many systems need get_magic_quotes_gpc() and it's related functions to get the information into a base line that all systems can then go from afterward.
dunc wrote:Make sure the variable contains only letters or only numbers, depending on variable type
It's not that concrete. The point of the commandment is mostly to tell you that you must always validate and verify data coming from outside sources. Sometimes even internal sources need to be checked. This is part of the concept of defense-in-depth.
dunc wrote:What else would you suggest?
Reading all the threads in the Security & Theory and Design boards.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

What else would you suggest?
I can highly recommend 2 books:
Essential PHP Security from Chris Shiflett (see his site and the books site)
PHP Architects Guide to PHP security from Ilia Alshanetsky

And I second Feyd's suggestion for reading all threads in security and theory&design. (might be some work ....)
dunc
Forum Newbie
Posts: 13
Joined: Mon Sep 04, 2006 10:17 am

Post by dunc »

Thanks matthijs :) I'm definitely gonna purchase Chris's book, ordered it from Amazon.

I've scripted a bit of code for the session variables I'm using. Is this totally secure, and is it the briefest way of doing it?

Code: Select all

$username = 0;
if (ctype_alnum($username)
{
 $username = make_safe($_SESSION['username']);
}
else
{
 exit("Username invalid. Please login.");
}

Code: Select all

function make_safe($var)
{
   $var = mysql_real_escape_string($var);
   $var = addslashes(trim($var));
   return $var;
}
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Ignoring the weirdness of your variable and parse error choices, near blind calls to addslashes() and mysql_real_escape_string() is overkill. The latter is all that's needed, however in this case, even the call to make_safe() is useless. The reason I say this is because ctype_alnum() has already verified that the variable in question is alphanumeric. There are no security holes in alphanumerics alone that I'm aware of.
dunc
Forum Newbie
Posts: 13
Joined: Mon Sep 04, 2006 10:17 am

Post by dunc »

feyd wrote:Ignoring the weirdness of your variable and parse error choices, near blind calls to addslashes() and mysql_real_escape_string() is overkill. The latter is all that's needed, however in this case, even the call to make_safe() is useless. The reason I say this is because ctype_alnum() has already verified that the variable in question is alphanumeric. There are no security holes in alphanumerics alone that I'm aware of.
For at least this page, all of the information being received will be alpha numeric - does that mean that using ctype_alnum will suffice for variable security?

What is wrong/wierd about my variable and parse error choices? :)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

dunc wrote:What is wrong/wierd about my variable and parse error choices?
Well, the posted snippet has a parse error.

After that, it has a logic error in that your ctype call is on a variable you just created, not the submitted variable that needs checking.
dunc
Forum Newbie
Posts: 13
Joined: Mon Sep 04, 2006 10:17 am

Post by dunc »

God I'm sorry; it's as if I hadn't even looked at the code :roll:

Code: Select all

$username = 0;
if (ctype_alnum($_SESSION['username']))
{
 $username = make_safe($_SESSION['username']);
}
else
{
 exit("Username invalid. Please login.");
}
But, if the alphanum check is sufficient:

Code: Select all

$username = 0;
if (ctype_alnum($_SESSION['username']))
{
 $username = $_SESSION['username'];
}
else
{
 unset($_SESSION['username']);
 header("Location: http://www.url.com/login.php");
}
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

I can only advise to call http://www.php.net/session_write_close before the header/redirect call...
mikesmith76
Forum Commoner
Posts: 34
Joined: Fri Aug 25, 2006 7:10 am
Location: Manchester, UK

Post by mikesmith76 »

I know i'm a newbie here but i'd like to express an opinion
even the call to make_safe() is useless
Now i know fyed has tonnes more experience than myself but imho your call to make_safe what not useless, it was redundant, and i don't think there is anything wrong with redundant error checking. Say in the future a php developer working on the ctype_alnum function makes an error, rendering the function useless (unlikely i know). If you relied on ctype_alnum as your sole method of defence your script may now be vunerable, for example to sql injection attacks. However the second call to make_safe() would probably prevent this attack. These is an extreme case but it serves to highlight the point I am trying to make

Don't get me wrong, i'm not encurraging anyone to call four or 5 different validation functions on any data. However i do commonly use two levels of validation, usually one of the ctype functions and then a call to a custom escape_data() function, on any data i use for my db.

Anyone have any thoughts on this?
Mike
Charles256
DevNet Resident
Posts: 1375
Joined: Fri Sep 16, 2005 9:06 pm

Post by Charles256 »

what if the functions he used in make_Safe were rendered useless by a programmers error? ;) what if the world blew up ? that's all I got to say.honest.
mikesmith76
Forum Commoner
Posts: 34
Joined: Fri Aug 25, 2006 7:10 am
Location: Manchester, UK

Post by mikesmith76 »

what if the functions he used in make_Safe were rendered useless by a programmers error?
My point wasn't that his make_safe function is fool proof, it was that i personally do not rely on one single method of validation. You are free to rely on whatever you want.
what if the world blew up ? that's all I got to say.honest.
Thank you for your constructive addition to the post. It really made me stop and think :-)

Does anyone else have a viewpoint on this?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

mikesmith76 wrote:Now i know fyed has tonnes more experience than myself but imho your call to make_safe what not useless, it was redundant, and i don't think there is anything wrong with redundant error checking. Say in the future a php developer working on the ctype_alnum function makes an error, rendering the function useless (unlikely i know). If you relied on ctype_alnum as your sole method of defence your script may now be vunerable, for example to sql injection attacks. However the second call to make_safe() would probably prevent this attack. These is an extreme case but it serves to highlight the point I am trying to make
In this particular case, make_safe will never alter the input after it's been checked ctype_alnum(). There's nothing for them to escape in alphanumerics. Yes, the functions used in make_safe() are redundant. The make_safe() itself did no checking, only blind transformation.

Regarding SQL injection, there is no such possibility if the data passes a call to ctype_alnum(). It's alphanumeric, nothing more. If you can make a real SQL injection that passes ctype_alnum(), I'll rescind my statement.
mikesmith76
Forum Commoner
Posts: 34
Joined: Fri Aug 25, 2006 7:10 am
Location: Manchester, UK

Post by mikesmith76 »

In this particular case, make_safe will never alter the input after it's been checked ctype_alnum(). There's nothing for them to escape in alphanumerics. Yes, the functions used in make_safe() are redundant. The make_safe() itself did no checking, only blind transformation.

Regarding SQL injection, there is no such possibility if the data passes a call to ctype_alnum(). It's alphanumeric, nothing more. If you can make a real SQL injection that passes ctype_alnum(), I'll rescind my statement.
I agree with everything you say here. In this particular case if ctype_alnum does its job and a security hole is never found in it then the call to make_safe will not change the data. However what i am trying to discover is if in general do other developers rely solely on one method of validation for input data, or do you all include redundant checking "just in case". I probably should've made my points clearer in my initial reply, and I apologise for any confusion.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

I have many layers of security in the code I build. Going from general to more and more strict the deeper it is. While I hate blindly running transforms, I do run mysql_real_escape_string() or whatever is native to the database in question on all the inputs rather than writing code to identify when and when not to do such things. I generally avoid addslashes() like the plague because it does have a security hole, but also isn't specific; it's a general function.
Post Reply