Page 1 of 2

Receiving information securely from forms

Posted: Tue Sep 05, 2006 8:26 am
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?

Re: Receiving information securely from forms

Posted: Tue Sep 05, 2006 8:35 am
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.

Posted: Tue Sep 05, 2006 11:05 am
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 ....)

Posted: Tue Sep 05, 2006 6:46 pm
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;
}

Posted: Tue Sep 05, 2006 6:56 pm
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.

Posted: Tue Sep 05, 2006 7:15 pm
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? :)

Posted: Tue Sep 05, 2006 7:20 pm
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.

Posted: Tue Sep 05, 2006 7:29 pm
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");
}

Posted: Wed Sep 06, 2006 12:39 am
by timvw
I can only advise to call http://www.php.net/session_write_close before the header/redirect call...

Posted: Wed Sep 06, 2006 2:31 am
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

Posted: Wed Sep 06, 2006 8:00 am
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.

Posted: Wed Sep 06, 2006 11:43 am
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?

Posted: Wed Sep 06, 2006 2:21 pm
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.

Posted: Wed Sep 06, 2006 4:29 pm
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.

Posted: Wed Sep 06, 2006 4:45 pm
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.