[Fwd: Re: [PHP] Explanation in Shiflett's PHP Security Briefing]
Hm? Didn't see this one yesterday on the list? Let's try again :) -afan Chris Shiflett wrote: You forgot to filter your input. Shame! :-) Escaping alone can save you in many cases, but always filter input and escape output. I confess: I didn't forget. I did it just wrong :( Even I thought I did understand - I didn't. I mixed filtering and escaping. :) Hope that helps. Oh, yeah! I just learned something new/more! Thanks for all of you guys! :) -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Explanation in Shiflett's PHP Security Briefing
First, thanks guys for such a fast response :)
Matthew Weier O'Phinney wrote:
While the above would prevent most SQL injections, it could still wreak
havoc with your database. For instance, what if your 'phone' or 'zip'
fields in your database are integer fields, and text gets passed from
the form? (answer: a failed DB call) Or you get a string of random
characters for the email? do you really want that in your DB?
I didn't mention anything about validating email address, zip code and
such a things, because it is not the issue. but, you are definitlly
right about how important are those.
Regarding your original question, the reason Chris S. keeps things in an
array is so that all CLEAN (i.e. valid and/or secure) data is marked as
such in a single place. Additionally, it allows you to do things like
validating your $_POST array by looping over it:
$clean = array();
foreach ($_POST as $key => $val) {
$ok = false;
switch ($key) {
case 'name':
if (ctype_alnum($val)) {
$ok = true;
}
break;
case 'address':
if (preg_match('/^[ a-z0-9.\'\"#-]+$/', $val)) {
$ok = true;
}
break;
// etc.
}
if ($ok) {
$clean[$key] = $val;
}
}
I agree with this one. It's definitlly more "clean" solution. :)
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Explanation in Shiflett's PHP Security Briefing
[EMAIL PROTECTED] wrote:
I got the point Chris was making: never believe _GET/_POST and use
ctype_alnum(), mysql_real_escape_string(), htmlentities() - and I
already started :) (Thanks Chris that was great for us beginners,
already posted on few Bosnian php forums :))
You're welcome. :-)
I must point out that the functions you just mentioned fall into two
separate categories.
Filtering:
ctype_alnum
Escaping:
htmlentities()
mysql_real_escape_string()
My question though was is the difference in code I mentioned just a
"habit of writing code" or there is some more? Some security issues too?
Well, the discipline of web application security involves both practical
and theoretical aspects. For example, the following two code snippets
are not equal:
Hi, {$_POST['username']}!";
}
?>
Hi, {$html['username']}!";
}
?>
They both do the exact same thing - they welcome a user according to the
username provided in a POST request. The difference is not a technical
one - it's a difference in theory.
By storing only filtered data in $clean, and by storing only filtered
data that has also been escaped for use in HTML in $html, I know that I
can safely send the value of any element within the $html array to the
client (browser).
This difference is hard to appreciate with such a simplistic example,
especially when the code is not spread out, but it mitigates the damage
that can be done when a developer makes a mistake. The worst-case
scenario is that $html['username'] does not exist - this is better than
it being tainted (both can be caused by the same mistake).
A developer can always output $_POST['username'] raw, but the idea is
that you can modify your approach to make things as easy as possible for
the security-conscious developers.
(This idea is also why I never recommend storing filtered data back into
$_POST - you want to foster the natural suspicion that developers have
for data stored therein, not deteriorate it.)
Let's try this way: we have a case of a form for storing registrant info
in DB. After submitting we have
$_POST['name']
$_POST['address']
$_POST['city']
$_POST['state']
$_POST['zip']
$_POST['email']
$_POST['phone']
To store submitted info to DB I would (now) use following code:
$name = mysql_real_escape_string($_POST['name']);
$address = mysql_real_escape_string($_POST['address']);
$city = mysql_real_escape_string($_POST['city']);
$state = mysql_real_escape_string($_POST['state']);
$zip = mysql_real_escape_string($_POST['zip']);
$email = mysql_real_escape_string($_POST['email']);
$phone = mysql_real_escape_string($_POST['phone']);
mysql_query("insert into info values (NULL, '$name', '$address',
'$city', '$state', '$email', '$phone')");
You forgot to filter your input. Shame! :-)
Escaping alone can save you in many cases, but always filter input and
escape output.
Hope that helps.
Chris
--
Chris Shiflett
Brain Bulb, The PHP Consultancy
http://brainbulb.com/
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Explanation in Shiflett's PHP Security Briefing
* "[EMAIL PROTECTED]" <[EMAIL PROTECTED]> :
> Thanks Richard.
> I got the point Chris was making: never believe _GET/_POST and use
> ctype_alnum(), mysql_real_escape_string(), htmlentities() - and I
> already started :) (Thanks Chris that was great for us beginners,
> already posted on few Bosnian php forums :))
>
> My question though was is the difference in code I mentioned just a
> "habit of writing code" or there is some more? Some security issues too?
>
> Let's try this way: we have a case of a form for storing registrant info
> in DB. After submitting we have
> $_POST['name']
> $_POST['address']
> $_POST['city']
> $_POST['state']
> $_POST['zip']
> $_POST['email']
> $_POST['phone']
>
> To store submitted info to DB I would (now) use following code:
>
> $name = mysql_real_escape_string($_POST['name']);
> $address = mysql_real_escape_string($_POST['address']);
> $city = mysql_real_escape_string($_POST['city']);
> $state = mysql_real_escape_string($_POST['state']);
> $zip = mysql_real_escape_string($_POST['zip']);
> $email = mysql_real_escape_string($_POST['email']);
> $phone = mysql_real_escape_string($_POST['phone']);
>
> mysql_query("insert into info values (NULL, '$name', '$address',
> '$city', '$state', '$email', '$phone')");
>
> doing the same using arrays:
>
> $submitted = array();
> $submitted['name'] = mysql_real_escape_string($_POST['name']);
> $submitted['address'] = mysql_real_escape_string($_POST['address']);
> $submitted['city'] = mysql_real_escape_string($_POST['city']);
> $submitted['state'] = mysql_real_escape_string($_POST['state']);
> $submitted['zip'] = mysql_real_escape_string($_POST['zip']);
> $submitted['email'] = mysql_real_escape_string($_POST['email']);
> $submitted['phone'] = mysql_real_escape_string($_POST['phone']);
>
> mysql_query("insert into info values (NULL, $submitted['name']',
> '$submitted['address']', '$submitted['city']', '$submitted['state']',
> '$submitted['zip']', '$submitted['email']', '$submitted['phone']')");
>
>
> Is this REALLY the same or there is a difference in security or
> something else?
Not the same, not necessarily secure, and it could cause issues for your
database and/or reporting.
What Chris Shifflet is getting at in his security columns is that simply
escaping your data before throwing it in the DB is not a good practice.
This is true from both a security standpoint as well as a database
management standpoint.
While the above would prevent most SQL injections, it could still wreak
havoc with your database. For instance, what if your 'phone' or 'zip'
fields in your database are integer fields, and text gets passed from
the form? (answer: a failed DB call) Or you get a string of random
characters for the email? do you really want that in your DB?
It's easy to get lazy about this stuff, especially when it's just a
hobby or for something non-critical, but if you get into bad habits,
what happens when you're doing this for a business critical application?
(I know this one -- my DBA starts beating me over the head with a bat
for f***ing up his data, which will now take him several days to fix; I
get fired from my job because a script I wrote allowed a hacker to
delete all our customer data; etc.)
Regarding your original question, the reason Chris S. keeps things in an
array is so that all CLEAN (i.e. valid and/or secure) data is marked as
such in a single place. Additionally, it allows you to do things like
validating your $_POST array by looping over it:
$clean = array();
foreach ($_POST as $key => $val) {
$ok = false;
switch ($key) {
case 'name':
if (ctype_alnum($val)) {
$ok = true;
}
break;
case 'address':
if (preg_match('/^[ a-z0-9.\'\"#-]+$/', $val)) {
$ok = true;
}
break;
// etc.
}
if ($ok) {
$clean[$key] = $val;
}
}
> Richard Davey wrote:
> > Monday, June 6, 2005, 6:39:09 PM, you wrote:
> > aan> I was reading PHP Security Briefing from brainbulb.com (Chris Shiflett)
> > aan> and didn't get one thing:
> > aan> in example:
> >
> > aan> > aan> $clean = array();
> > aan> if (ctype_alnum($_POST['username']))
> > aan> {
> > aan> $clean['username'] = $_POST['username'];
> > aan> }
> > ?> >
> >
> > aan> why to set the $clean as array? what's wrong if I use:
> >
> > aan> > aan> if (ctype_alnum($_POST['username']))
> > aan> {
> > aan> $clean = $_POST['username'];
> > aan> }
> > ?> >
> >
> > In your example $clean will only ever hold one value. In the original
> > the clean array can be used to hold all clean GET/POST values. Not
> > many forms only have one value. The most important thing to remember
> > though is that your array isn't really "clean", it's just "valid". I
> > believe the original point Chris was making was that you should never
> > trust that $_POST will only contain the values you expect it to - they
> > should be moved o
Re: [PHP] Explanation in Shiflett's PHP Security Briefing
[EMAIL PROTECTED] wrote: I was reading PHP Security Briefing from brainbulb.com (Chris Shiflett) and didn't get one thing: in example: why to set the $clean as array? what's wrong if I use: Richard already answered this pretty well, but I wanted to mention that this is not the only way to do things - it's just the method that I use. The idea is that I have a single variable ($clean) that I make sure to initialize, and because it's an array, I can store all filtered data (and only filtered data) in it. You could use a naming convention like $clean_username, but then you would have numerous variables to initialize on each page, increasing the chances of you forgetting one (although E_ALL can help you catch these mistakes). I'm just going for the simplest approach - the easier I can make things, the less likely I am to make a mistake. :-) The bottom line is that you need to be able to easily and reliably distinguish between filtered and tainted data. How you do this is up to you. Hope that helps. Chris -- Chris Shiflett Brain Bulb, The PHP Consultancy http://brainbulb.com/ -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Explanation in Shiflett's PHP Security Briefing
Thanks Richard.
I got the point Chris was making: never believe _GET/_POST and use
ctype_alnum(), mysql_real_escape_string(), htmlentities() - and I
already started :) (Thanks Chris that was great for us beginners,
already posted on few Bosnian php forums :))
My question though was is the difference in code I mentioned just a
"habit of writing code" or there is some more? Some security issues too?
Let's try this way: we have a case of a form for storing registrant info
in DB. After submitting we have
$_POST['name']
$_POST['address']
$_POST['city']
$_POST['state']
$_POST['zip']
$_POST['email']
$_POST['phone']
To store submitted info to DB I would (now) use following code:
$name = mysql_real_escape_string($_POST['name']);
$address = mysql_real_escape_string($_POST['address']);
$city = mysql_real_escape_string($_POST['city']);
$state = mysql_real_escape_string($_POST['state']);
$zip = mysql_real_escape_string($_POST['zip']);
$email = mysql_real_escape_string($_POST['email']);
$phone = mysql_real_escape_string($_POST['phone']);
mysql_query("insert into info values (NULL, '$name', '$address',
'$city', '$state', '$email', '$phone')");
doing the same using arrays:
$submitted = array();
$submitted['name'] = mysql_real_escape_string($_POST['name']);
$submitted['address'] = mysql_real_escape_string($_POST['address']);
$submitted['city'] = mysql_real_escape_string($_POST['city']);
$submitted['state'] = mysql_real_escape_string($_POST['state']);
$submitted['zip'] = mysql_real_escape_string($_POST['zip']);
$submitted['email'] = mysql_real_escape_string($_POST['email']);
$submitted['phone'] = mysql_real_escape_string($_POST['phone']);
mysql_query("insert into info values (NULL, $submitted['name']',
'$submitted['address']', '$submitted['city']', '$submitted['state']',
'$submitted['zip']', '$submitted['email']', '$submitted['phone']')");
Is this REALLY the same or there is a difference in security or
something else?
Thanks :)
-afan
Richard Davey wrote:
Hello afan,
Monday, June 6, 2005, 6:39:09 PM, you wrote:
aan> I was reading PHP Security Briefing from brainbulb.com (Chris Shiflett)
aan> and didn't get one thing:
aan> in example:
aan> $clean = array();
aan> if (ctype_alnum($_POST['username']))
aan> {
aan> $clean['username'] = $_POST['username'];
aan> }
?>>
aan> why to set the $clean as array? what's wrong if I use:
aan> if (ctype_alnum($_POST['username']))
aan> {
aan> $clean = $_POST['username'];
aan> }
?>>
In your example $clean will only ever hold one value. In the original
the clean array can be used to hold all clean GET/POST values. Not
many forms only have one value. The most important thing to remember
though is that your array isn't really "clean", it's just "valid". I
believe the original point Chris was making was that you should never
trust that $_POST will only contain the values you expect it to - they
should be moved out into a clean array first for further inspection
and filtering, if anything else lingers in the $_POST array, it's most
likely been tainted.
Best regards,
Richard Davey
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Explanation in Shiflett's PHP Security Briefing
Hello afan,
Monday, June 6, 2005, 6:39:09 PM, you wrote:
aan> I was reading PHP Security Briefing from brainbulb.com (Chris Shiflett)
aan> and didn't get one thing:
aan> in example:
aan> $clean = array();
aan> if (ctype_alnum($_POST['username']))
aan> {
aan> $clean['username'] = $_POST['username'];
aan> }
?>>
aan> why to set the $clean as array? what's wrong if I use:
aan> if (ctype_alnum($_POST['username']))
aan> {
aan> $clean = $_POST['username'];
aan> }
?>>
In your example $clean will only ever hold one value. In the original
the clean array can be used to hold all clean GET/POST values. Not
many forms only have one value. The most important thing to remember
though is that your array isn't really "clean", it's just "valid". I
believe the original point Chris was making was that you should never
trust that $_POST will only contain the values you expect it to - they
should be moved out into a clean array first for further inspection
and filtering, if anything else lingers in the $_POST array, it's most
likely been tainted.
Best regards,
Richard Davey
--
http://www.launchcode.co.uk - PHP Development Services
"I do not fear computers. I fear the lack of them." - Isaac Asimov
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
