Post input processing and security

In addition to validating post input to see if it has been entered or
confroms to a regular expression, I am interested in secure ways to
efficiently process incomiing form data.

I use a function to process this data, namely:

function escape_data ($data) {

if (ini_get('magic_quotes_gpc')) {
$data = stripslashes($data);
}
return mysql_real_escape_string(strip_tags(trim(urldecode($data)))) ;
}

It's tempting to apply it to all incoming data at once, like this:

foreach($_POST as $key => $value) {
global $$key;
$$key = escape_data($value);

}

Of course this introduces the well known vulnerability of putting the
data into variables that can be modified.

Another way to do this is to put the data in an array, like this:

foreach($_POST as $key => $value) {
global $input;
$input[$key] = escape_data($value);

}

This has the advantage that I can use implode() to create strings that
can be applied to a database query.

However, it includes all post variables, including data I dont' intend
to insert in my database so I can be selective about what form
elements I include like this:

function ($namelist) {

for each ($namelist as $formdata) {
if (!empty($_POST[$formdata]) {
global $formdata;
$formdata[] = escape_data($_POST[$formdata]);
}
}
}

$namelist being an array of the names of the form elements I want to
extract for my query.

I'm interested on people's reflections on the security of these
methods as well as other comments and suggestions for other
approaches.

Thanks.

--Kenoli
kenoli [ Mo, 09 April 2007 02:26 ] [ ID #1682466 ]

Re: Post input processing and security

ken...

have you thought of using array_walk?

function escapeData(&$value)
{
if (ini_get('magic_quotes_gpc'))
{
$value = stripslashes($value);
}
$value = mysql_real_escape_string(strip_tags(trim(urldecode($value))) );
}

array_walk($_POST, escapeData);


since the function is compiled into php, it will run natively and is
therefore much faster than using your own 'for' loop to iterate the post
array. that may be something for you to think about.

as for escapeData, you can cause yourself headaches by assuming all data is
passed as urlencoded. it is not needed as far as i'm concerned...i only
urlencode when i'm dynamically creating a hyperlink for html. php typically
presents you with the decoded version of data (so to speak). you would be
forcing user input that had a space character to be encoded as a plus sign
(+) from the client to the server, and then force the server to translate it
back to a space. i'm not sure why you'd do this.

oh, and also, you can run into problems casting escaped data into things
like dates and currency. better to allow a less broad stroke to handle
things...that means, be more delicate.

another issue would be stripping tags as some sort of default (with no
option to override it). i may be asked on your form to enter my email
address. if i put:

Joe Blow <jblow [at] example.com>

my address would be valid, but you're code would be potentially stripping
out the portion of it that is the actual valid piece of data...and again,
you provide no means to override that functionality.

as for doing $$key as a global from the post array...BAD IDEA !!! most
obvious is the introduction of an opportunity for a hacker to overright
predicted variables (userName, isValid, isSecure, etc.). the not-so-obvious
downfall is that your variables are not explicitly set, transparent to other
developers...including yourself when you come back to edit later. they'd be
saying, 'where does this variable come from? from an included file, from a
post, get, or other means? has it been set already, or is this where it is
being defined?' that list goes on...and really pisses people off. be
explicit at all times so that no one has to guess an how things work.

your last idea is probably a good compromise. again though, you may want to
simplify your code and speed it up using array_walk. also reuse namelist to
help build your forms. namelist should not only define the variable name but
what data-type it is supposed to be, what validation applies to it,
max-length, input-type (text, hidden, select, etc.), and value. what you
have now is the criterion for a class object. your namelist is an array of
those objects and instead of just escaping data, you can set the value
either to a default when empty or to the actual posted data. further, you
can apply the validation and generate errors for the end-user. even more
than that, you can walk the namelist array to generate your form with less
lines of code...which means changes (enhancements, bug-fixes, etc.) are
isolated to one section of code. AND, you've now been *very* explicit as to
what the data means, where it comes from, and how it fits in to the
application as a whole. no one being left confused. and, do you also see
that this would allow you to now catch that the data is a date, currency,
float, or other where you could now not just escape the string but use
set_type() on the value to avoid problems later?

one final remark...NEVER, EVER use $global. until you've learned the
condition(s) under which it is a good idea, 'never' is literal...otherwise,
you'll know that i really mean, 'only when it is absolutely
necessary...which is almost never'.

;^)

hth,

me


"kenoli" <kenoli.p [at] gmail.com> wrote in message
news:1176078404.793386.307530 [at] w1g2000hsg.googlegroups.com...
| In addition to validating post input to see if it has been entered or
| confroms to a regular expression, I am interested in secure ways to
| efficiently process incomiing form data.
|
| I use a function to process this data, namely:
|
| function escape_data ($data) {
|
| if (ini_get('magic_quotes_gpc')) {
| $data = stripslashes($data);
| }
| return mysql_real_escape_string(strip_tags(trim(urldecode($data)))) ;
| }
|
| It's tempting to apply it to all incoming data at once, like this:
|
| foreach($_POST as $key => $value) {
| global $$key;
| $$key = escape_data($value);
|
| }
|
| Of course this introduces the well known vulnerability of putting the
| data into variables that can be modified.
|
| Another way to do this is to put the data in an array, like this:
|
| foreach($_POST as $key => $value) {
| global $input;
| $input[$key] = escape_data($value);
|
| }
|
| This has the advantage that I can use implode() to create strings that
| can be applied to a database query.
|
| However, it includes all post variables, including data I dont' intend
| to insert in my database so I can be selective about what form
| elements I include like this:
|
| function ($namelist) {
|
| for each ($namelist as $formdata) {
| if (!empty($_POST[$formdata]) {
| global $formdata;
| $formdata[] = escape_data($_POST[$formdata]);
| }
| }
| }
|
| $namelist being an array of the names of the form elements I want to
| extract for my query.
|
| I'm interested on people's reflections on the security of these
| methods as well as other comments and suggestions for other
| approaches.
|
| Thanks.
|
| --Kenoli
|
Steve [ Mo, 09 April 2007 03:53 ] [ ID #1682467 ]

Re: Post input processing and security

Steve -- thanks so much for your suggestions. I had earlier
considered array_walk and your reasoning for using it makes sense.
Also thanks for your suggestion about being more discriminating about
what string functions I apply to what. Your insight into the way the
the $$key approach invisibly creates a lof of variables that might not
be evident to someone, even me, returning to the script at a later
date is a good insight. It has stumped me in the past.

I have been thinking about creating a class for form processing of the
sort you describe as an exercise in understanding OOP since I find
myself re-inventing the wheel with forms and queries so often. I
suppose s sophisticaed class script could even check out what database
columns are being used and match them to the form imputs to generate
various kinds of queries. I have yet to actually utilize a class or
object, though I have read up on it; I still feel a bit intimidated.
I guess I need to jump in. I haven't come across a class that does
this sort of thing related to forms, which surprises me as, in my
coding, it is one of the things I end up coding and re-coding most
often.

--Kenoli

On Apr 8, 6:53 pm, "Steve" <no.... [at] example.com> wrote:
> ken...
>
> have you thought of using array_walk?
>
> function escapeData(&$value)
> {
> if (ini_get('magic_quotes_gpc'))
> {
> $value = stripslashes($value);
> }
> $value = mysql_real_escape_string(strip_tags(trim(urldecode($value))) );
>
> }
>
> array_walk($_POST, escapeData);
>
> since the function is compiled into php, it will run natively and is
> therefore much faster than using your own 'for' loop to iterate the post
> array. that may be something for you to think about.
>
> as for escapeData, you can cause yourself headaches by assuming all data is
> passed as urlencoded. it is not needed as far as i'm concerned...i only
> urlencode when i'm dynamically creating a hyperlink for html. php typically
> presents you with the decoded version of data (so to speak). you would be
> forcing user input that had a space character to be encoded as a plus sign
> (+) from the client to the server, and then force the server to translate it
> back to a space. i'm not sure why you'd do this.
>
> oh, and also, you can run into problems casting escaped data into things
> like dates and currency. better to allow a less broad stroke to handle
> things...that means, be more delicate.
>
> another issue would be stripping tags as some sort of default (with no
> option to override it). i may be asked on your form to enter my email
> address. if i put:
>
> Joe Blow <j... [at] example.com>
>
> my address would be valid, but you're code would be potentially stripping
> out the portion of it that is the actual valid piece of data...and again,
> you provide no means to override that functionality.
>
> as for doing $$key as a global from the post array...BAD IDEA !!! most
> obvious is the introduction of an opportunity for a hacker to overright
> predicted variables (userName, isValid, isSecure, etc.). the not-so-obvious
> downfall is that your variables are not explicitly set, transparent to other
> developers...including yourself when you come back to edit later. they'd be
> saying, 'where does this variable come from? from an included file, from a
> post, get, or other means? has it been set already, or is this where it is
> being defined?' that list goes on...and really pisses people off. be
> explicit at all times so that no one has to guess an how things work.
>
> your last idea is probably a good compromise. again though, you may want to
> simplify your code and speed it up using array_walk. also reuse namelist to
> help build your forms. namelist should not only define the variable name but
> what data-type it is supposed to be, what validation applies to it,
> max-length, input-type (text, hidden, select, etc.), and value. what you
> have now is the criterion for a class object. your namelist is an array of
> those objects and instead of just escaping data, you can set the value
> either to a default when empty or to the actual posted data. further, you
> can apply the validation and generate errors for the end-user. even more
> than that, you can walk the namelist array to generate your form with less
> lines of code...which means changes (enhancements, bug-fixes, etc.) are
> isolated to one section of code. AND, you've now been *very* explicit as to
> what the data means, where it comes from, and how it fits in to the
> application as a whole. no one being left confused. and, do you also see
> that this would allow you to now catch that the data is a date, currency,
> float, or other where you could now not just escape the string but use
> set_type() on the value to avoid problems later?
>
> one final remark...NEVER, EVER use $global. until you've learned the
> condition(s) under which it is a good idea, 'never' is literal...otherwise,
> you'll know that i really mean, 'only when it is absolutely
> necessary...which is almost never'.
>
> ;^)
>
> hth,
>
> me
>
> "kenoli" <kenol... [at] gmail.com> wrote in message
>
> news:1176078404.793386.307530 [at] w1g2000hsg.googlegroups.com...
> | In addition to validating post input to see if it has been entered or
> | confroms to a regular expression, I am interested in secure ways to
> | efficiently process incomiing form data.
> |
> | I use a function to process this data, namely:
> |
> | function escape_data ($data) {
> |
> | if (ini_get('magic_quotes_gpc')) {
> | $data = stripslashes($data);
> | }
> | return mysql_real_escape_string(strip_tags(trim(urldecode($data)))) ;
> | }
> |
> | It's tempting to apply it to all incoming data at once, like this:
> |
> | foreach($_POST as $key => $value) {
> | global $$key;
> | $$key = escape_data($value);
> |
> | }
> |
> | Of course this introduces the well known vulnerability of putting the
> | data into variables that can be modified.
> |
> | Another way to do this is to put the data in an array, like this:
> |
> | foreach($_POST as $key => $value) {
> | global $input;
> | $input[$key] = escape_data($value);
> |
> | }
> |
> | This has the advantage that I can use implode() to create strings that
> | can be applied to a database query.
> |
> | However, it includes all post variables, including data I dont' intend
> | to insert in my database so I can be selective about what form
> | elements I include like this:
> |
> | function ($namelist) {
> |
> | for each ($namelist as $formdata) {
> | if (!empty($_POST[$formdata]) {
> | global $formdata;
> | $formdata[] = escape_data($_POST[$formdata]);
> | }
> | }
> | }
> |
> | $namelist being an array of the names of the form elements I want to
> | extract for my query.
> |
> | I'm interested on people's reflections on the security of these
> | methods as well as other comments and suggestions for other
> | approaches.
> |
> | Thanks.
> |
> | --Kenoli
> |
kenoli [ Mo, 09 April 2007 23:34 ] [ ID #1682480 ]

Re: Post input processing and security

no problem.

how's the site been coming along?


"kenoli" <kenoli.p [at] gmail.com> wrote in message
news:1176154470.232600.284480 [at] n59g2000hsh.googlegroups.com.. .
| Steve -- thanks so much for your suggestions. I had earlier
| considered array_walk and your reasoning for using it makes sense.
| Also thanks for your suggestion about being more discriminating about
| what string functions I apply to what. Your insight into the way the
| the $$key approach invisibly creates a lof of variables that might not
| be evident to someone, even me, returning to the script at a later
| date is a good insight. It has stumped me in the past.
|
| I have been thinking about creating a class for form processing of the
| sort you describe as an exercise in understanding OOP since I find
| myself re-inventing the wheel with forms and queries so often. I
| suppose s sophisticaed class script could even check out what database
| columns are being used and match them to the form imputs to generate
| various kinds of queries. I have yet to actually utilize a class or
| object, though I have read up on it; I still feel a bit intimidated.
| I guess I need to jump in. I haven't come across a class that does
| this sort of thing related to forms, which surprises me as, in my
| coding, it is one of the things I end up coding and re-coding most
| often.
|
| --Kenoli
|
| On Apr 8, 6:53 pm, "Steve" <no.... [at] example.com> wrote:
| > ken...
| >
| > have you thought of using array_walk?
| >
| > function escapeData(&$value)
| > {
| > if (ini_get('magic_quotes_gpc'))
| > {
| > $value = stripslashes($value);
| > }
| > $value =
mysql_real_escape_string(strip_tags(trim(urldecode($value))) );
| >
| > }
| >
| > array_walk($_POST, escapeData);
| >
| > since the function is compiled into php, it will run natively and is
| > therefore much faster than using your own 'for' loop to iterate the post
| > array. that may be something for you to think about.
| >
| > as for escapeData, you can cause yourself headaches by assuming all data
is
| > passed as urlencoded. it is not needed as far as i'm concerned...i only
| > urlencode when i'm dynamically creating a hyperlink for html. php
typically
| > presents you with the decoded version of data (so to speak). you would
be
| > forcing user input that had a space character to be encoded as a plus
sign
| > (+) from the client to the server, and then force the server to
translate it
| > back to a space. i'm not sure why you'd do this.
| >
| > oh, and also, you can run into problems casting escaped data into things
| > like dates and currency. better to allow a less broad stroke to handle
| > things...that means, be more delicate.
| >
| > another issue would be stripping tags as some sort of default (with no
| > option to override it). i may be asked on your form to enter my email
| > address. if i put:
| >
| > Joe Blow <j... [at] example.com>
| >
| > my address would be valid, but you're code would be potentially
stripping
| > out the portion of it that is the actual valid piece of data...and
again,
| > you provide no means to override that functionality.
| >
| > as for doing $$key as a global from the post array...BAD IDEA !!! most
| > obvious is the introduction of an opportunity for a hacker to overright
| > predicted variables (userName, isValid, isSecure, etc.). the
not-so-obvious
| > downfall is that your variables are not explicitly set, transparent to
other
| > developers...including yourself when you come back to edit later. they'd
be
| > saying, 'where does this variable come from? from an included file, from
a
| > post, get, or other means? has it been set already, or is this where it
is
| > being defined?' that list goes on...and really pisses people off. be
| > explicit at all times so that no one has to guess an how things work.
| >
| > your last idea is probably a good compromise. again though, you may want
to
| > simplify your code and speed it up using array_walk. also reuse namelist
to
| > help build your forms. namelist should not only define the variable name
but
| > what data-type it is supposed to be, what validation applies to it,
| > max-length, input-type (text, hidden, select, etc.), and value. what you
| > have now is the criterion for a class object. your namelist is an array
of
| > those objects and instead of just escaping data, you can set the value
| > either to a default when empty or to the actual posted data. further,
you
| > can apply the validation and generate errors for the end-user. even more
| > than that, you can walk the namelist array to generate your form with
less
| > lines of code...which means changes (enhancements, bug-fixes, etc.) are
| > isolated to one section of code. AND, you've now been *very* explicit as
to
| > what the data means, where it comes from, and how it fits in to the
| > application as a whole. no one being left confused. and, do you also see
| > that this would allow you to now catch that the data is a date,
currency,
| > float, or other where you could now not just escape the string but use
| > set_type() on the value to avoid problems later?
| >
| > one final remark...NEVER, EVER use $global. until you've learned the
| > condition(s) under which it is a good idea, 'never' is
literal...otherwise,
| > you'll know that i really mean, 'only when it is absolutely
| > necessary...which is almost never'.
| >
| > ;^)
| >
| > hth,
| >
| > me
| >
| > "kenoli" <kenol... [at] gmail.com> wrote in message
| >
| > news:1176078404.793386.307530 [at] w1g2000hsg.googlegroups.com...
| > | In addition to validating post input to see if it has been entered or
| > | confroms to a regular expression, I am interested in secure ways to
| > | efficiently process incomiing form data.
| > |
| > | I use a function to process this data, namely:
| > |
| > | function escape_data ($data) {
| > |
| > | if (ini_get('magic_quotes_gpc')) {
| > | $data = stripslashes($data);
| > | }
| > | return mysql_real_escape_string(strip_tags(trim(urldecode($data)))) ;
| > | }
| > |
| > | It's tempting to apply it to all incoming data at once, like this:
| > |
| > | foreach($_POST as $key => $value) {
| > | global $$key;
| > | $$key = escape_data($value);
| > |
| > | }
| > |
| > | Of course this introduces the well known vulnerability of putting the
| > | data into variables that can be modified.
| > |
| > | Another way to do this is to put the data in an array, like this:
| > |
| > | foreach($_POST as $key => $value) {
| > | global $input;
| > | $input[$key] = escape_data($value);
| > |
| > | }
| > |
| > | This has the advantage that I can use implode() to create strings that
| > | can be applied to a database query.
| > |
| > | However, it includes all post variables, including data I dont' intend
| > | to insert in my database so I can be selective about what form
| > | elements I include like this:
| > |
| > | function ($namelist) {
| > |
| > | for each ($namelist as $formdata) {
| > | if (!empty($_POST[$formdata]) {
| > | global $formdata;
| > | $formdata[] = escape_data($_POST[$formdata]);
| > | }
| > | }
| > | }
| > |
| > | $namelist being an array of the names of the form elements I want to
| > | extract for my query.
| > |
| > | I'm interested on people's reflections on the security of these
| > | methods as well as other comments and suggestions for other
| > | approaches.
| > |
| > | Thanks.
| > |
| > | --Kenoli
| > |
|
|
Steve [ Di, 10 April 2007 22:05 ] [ ID #1683488 ]
PHP » alt.php » Post input processing and security

Vorheriges Thema: radio button, check for use
Nächstes Thema: PHP + IIS + Visual Studio.NET 2005 and Apache