adding "addslahses" to already live project - best approach?

[Dave Smithz]

Having adopted someone else's PHP cope and completing a crash course in the
language I came across a (probably common) problem with the current code.

On a registration form, whenever users names have an apostrophe in them it
causes problems as they do not get added to the DB correctly for reasons
that immediately become apparent.

Before implementing my own workaround I noticed the functions.

addslashes, stripslashes and directive magic_quotes_gpc. These seem like
great ideas and I will now use them.

What is the best way of introducing this practise across all of the code for
this project. It is something I want to progressively, therefore as I make
changes when necessary, I will also add these changes, I do not want to
have to go through the entire code now changing everything.

Can I just do something like the following

On each new script I create do the following.

Check if the magic quotes directive is enabled, and if so do nothing, but if
not, manually run the addslashes function and wherever data is retrieved
from the database go in and add a stripslashes.

Actually I could just make the changes in the code that writes to and
retrieves info from the database rather then on each script that collects
form input. This would therefore lessen the need to make multiple changes to
the code.

Is that all I will need to do. Is there any implications that I am not aware
of by implementing this in a particular way?

Forgive the slight vagueness in this question, I am still surprised to find
this was not done in the start.

Kind regards

Dave

[Brion Vibber]

Dave Smithz wrote:

On each new script I create do the following.

Check if the magic quotes directive is enabled, and if so do nothing, but if
not, manually run the addslashes function and wherever data is retrieved
from the database go in and add a stripslashes.
You should never have to run stripslashes on data taken _out_ of the
database. If you do, either your data was corrupt going in or you have
the 'magic_quotes_runtime' option on. magic_quotes_runtime will corrupt
data unpredictably and you should probably never use it.

In my experience the most reliable thing to do is to always escape data
*exactly when putting it an SQL command*, not earlier. Not all form
input is dumped into raw SQL commands, and not all data destined for SQL
commands comes directly from form input, so magic_quotes_gpc is very
unreliable.

In particular, note that if you process text after it comes from a form
and before putting it into an SQL command, the pre-added slashes may be
corrupted or removed. If you relied on the original slashes to be
present, you may become vulnerable to an SQL injection attack.

What I end up doing is running stripslashes() on all the form input when
magic_quotes_gpc is on, so that data can be passed around within my
program in its proper state, then ensuring that all generated SQL
statement generators escape their arguments.
Actually I could just make the changes in the code that writes to and
retrieves info from the database rather then on each script that collects
form input. This would therefore lessen the need to make multiple changes to
the code.
You should always escape data before inserting it into a raw SQL
statement as a literal value. addslashes() may or may not be correct,
depending on your database; use the appropriate function for your
database (such as mysql_real_escape_string). Consider also PEAR::DB's
helper functions which can often perform this step for you.
Is that all I will need to do. Is there any implications that I am not aware
of by implementing this in a particular way?

Some things you don't want to do:
* insufficient escaping -> SQL injection
* processing strings after escaping -> SQL injection
* double escaping -> data corruption
* using SQL-escaped strings in a non-SQL context -> data corruption

-- brion vibber (brion @ pobox.com)

[Brent Palmer]

"Dave Smithz" <SPAM FREE WORLD> wrote in message
news:42********@news1.homechoice.co.uk...

Having adopted someone else's PHP cope and completing a crash course in
the
language I came across a (probably common) problem with the current code.

On a registration form, whenever users names have an apostrophe in them it
causes problems as they do not get added to the DB correctly for reasons
that immediately become apparent.

Before implementing my own workaround I noticed the functions.

addslashes, stripslashes and directive magic_quotes_gpc. These seem like
great ideas and I will now use them.

What is the best way of introducing this practise across all of the code
for
this project. It is something I want to progressively, therefore as I make
changes when necessary, I will also add these changes, I do not want to
have to go through the entire code now changing everything.

Can I just do something like the following

On each new script I create do the following.

Check if the magic quotes directive is enabled, and if so do nothing, but
if
not, manually run the addslashes function and wherever data is retrieved
from the database go in and add a stripslashes.

Actually I could just make the changes in the code that writes to and
retrieves info from the database rather then on each script that collects
form input. This would therefore lessen the need to make multiple changes
to
the code.

Is that all I will need to do. Is there any implications that I am not
aware
of by implementing this in a particular way?

Forgive the slight vagueness in this question, I am still surprised to
find
this was not done in the start.

Kind regards

Dave

I agree with Brian but I use the function htmlspecialchars();
This changes the special chars like quotes ' and " and so on... to HTML
entities .
Eg.
'&' (ampersand) becomes '&amp;'
'"' (double quote) becomes '&quot;' when ENT_NOQUOTES is not set.
''' (single quote) becomes ''' only when ENT_QUOTES is set.
http://au2.php.net/htmlspecialchars

Brent Palmer.

[Geoff Berrow]

I noticed that Message-ID: <42********@news1.homechoice.co.uk> from
"Dave Smithz" <SPAM FREE WORLD> contained the following:

<on measures that need to be taken to sanitise data input>

FAQ entry folks?

--
Geoff Berrow (put thecat out to email)
It's only Usenet, no one dies.
My opinions, not the committee's, mine.
Simple RFDs http://www.ckdog.co.uk/rfdmaker/

[Kristian Köhntopp]

"Dave Smithz" <SPAM FREE WORLD> wrote:

addslashes, stripslashes and directive magic_quotes_gpc. These seem like
great ideas and I will now use them.

Don't. Please use
http://www.php.net/manual/en/functio...ape-string.php or the
database specific function for the database you are using instead. Also,
there is no need to use stripslashes(), ever. Instead, the database will
remove the necessary quotes upon insert, and you will retrieve properly
unquoted data from the database.

On a related note, please check your code for input sanitation. Just like
you encapsulate database access using a database "abstraction" library, you
should encapsulate all data importing functionality from the user side of
your application and this encapsulation should perform strict sanity
checking on the data. That is, type and range should be enforced ("Is the
supposedly integer variable really containing a number?", "Is the select
input field actually returning a value from the available selection, or
something different?"), and data should be kept locally in your session
once it has been passed through the santiation once. Don't bounce data
using forms and hidden variables.

Kristian

[Brion Vibber]

Brent Palmer wrote:

I agree with Brian but I use the function htmlspecialchars();
This changes the special chars like quotes ' and " and so on... to HTML
entities .
Eg.
'&' (ampersand) becomes '&amp;'
'"' (double quote) becomes '&quot;' when ENT_NOQUOTES is not set.
''' (single quote) becomes ''' only when ENT_QUOTES is set.
http://au2.php.net/htmlspecialchars

htmlspecialchars() is a vital safety tool for putting text data into
HTML output. You should always use this (or htmlentities() or a similar
transformation) when outputting data to HTML to prevent JavaScript
injection attacks.

I hope you're not using it for constructing SQL statements, though.
Since it will pass other special characters such as the backslash, it
may be possible to create SQL injection attacks unless something else
protects against it.

For instance a query like:
SELECT field1,field2 FROM privatedata WHERE
username='O'Connor' and year='2005'

could be used to return data from all rows in the table by turning it into:
SELECT field1,field2 FROM privatedata WHERE
username='\' AND year=' OR 1=1 /*'

-- brion vibber (brion @ pobox.com)

[Daniel Tryba]

Geoff Berrow <bl******@ckdog.co.uk> wrote:

<on measures that need to be taken to sanitise data input>

FAQ entry folks?

Q. I'm getting extra \ in my form field
A. You are using Get/Post/Cookie (GPC) data without stripping the
magically [1] added quotes (which are on by default).
If aren't getting the data from GPC magic_quotes_runtime might be
on (off by default)

Q. When to escape?
A. Only use the right escape method at the moment it is needed.
What the right escape method actually is depends on where the data
will be used. If you are inserting the string $bar into eg mysql
you should escape it with mysql_real_escape_string() [2]
$query="UPDATE foo SET bar='".mysql_real_escape_string($bar)."'";
the same base shoule be htmlescaped [3] when used in html
echo "<a href='foo.php?bar=".urlencode($bar)."'>".htmlspeci alchars($bar,ENT_QUOTES)."</a>";
(also note that $bar needs to be urlescaped [4] if used in an URL)
So in oorder to make this work you should always keep the raw
unescaped values (this is why (IMHO) magic_quotes is evil). To make
sure you are actually working with the raw values you should sanitize
GPC data with something like this (untested and incomplete code):
function slashed($t)
{
if(get_magic_quotes_gpc())
{
if(is_array($t) && count($t))
for($i=0;$i<count($t);$i++)
$t[$i]=slashed($t[$i]);
else
$t=stripslashes($t);
}
return $t;
}
$_GET=slashed($_GET);
$_POST=slashed($_POST);
$_REQUEST=slashed($_REQUEST);
$_COOKIE=slashed($_COOKIE);

Q. When and how to sanitize data
A. Like escaping it depends on usage. If know data in a sql row should
be an int, you could do something like this:
$query="UPDATE foo SET bar='".((int)$bar)."'";
If the data should contain a dutch style postalcode:
if(!preg_match("/^\d{4}\s*[A-Za-z]{2}$/",trim($zip))
die("error in zip");
But IMHO you shouldn't try to fix obviously wrong data.

[1] http://nl2.php.net/manual/en/security.magicquotes.php
[2] http://nl2.php.net/manual/en/functio...ape-string.php
[3] http://nl2.php.net/manual/en/functio...ecialchars.php
[4] http://nl2.php.net/manual/en/function.urlencode.php

[R. Rajesh Jeba Anbiah]

Brion Vibber wrote:
<snip>

I hope you're not using it for constructing SQL statements, though.
Since it will pass other special characters such as the backslash, it may be possible to create SQL injection attacks unless something else protects against it.

For instance a query like:
SELECT field1,field2 FROM privatedata WHERE
username='O'Connor' and year='2005'

could be used to return data from all rows in the table by turning it into: SELECT field1,field2 FROM privatedata WHERE
username='\' AND year=' OR 1=1 /*'

Just curious, what's your suggestion to avoid this last case you
mentioned? Are you hinting to use
addslashes(htmlspecialchars($_POST['foo']))?

--
<?php echo 'Just another PHP saint'; ?>
Email: rrjanbiah-at-Y!com Blog: http://rajeshanbiah.blogspot.com/

[Brion Vibber]

R. Rajesh Jeba Anbiah wrote:

Brion Vibber wrote:

SELECT field1,field2 FROM privatedata WHERE
username='\' AND year=' OR 1=1 /*'

Just curious, what's your suggestion to avoid this last case you
mentioned? Are you hinting to use
addslashes(htmlspecialchars($_POST['foo']))?

Running htmlspecialchars() on material being put into a database seems
pretty odd to begin with; normally I'd expect to see that done on output.

Just use the correct escaping function for your database, such as
mysql_real_escape_string() or PEAR::DB's escapeSimple() method.
addslashes() may often work, but isn't guaranteed to be correct
depending on your database. htmlspecialchars() is never correct for this
purpose.

-- brion vibber (brion @ pobox.com)

[Dave Smithz]

"Kristian Köhntopp" <kr**@koehntopp.de> wrote in message
news:d1m74k$8rp$1@xn--abcdefghijklmnopqrstuvwxyzss-

Don't. Please use
http://www.php.net/manual/en/functio...ape-string.php or the
database specific function for the database you are using instead. Also,
there is no need to use stripslashes(), ever. Instead, the database will
remove the necessary quotes upon insert, and you will retrieve properly
unquoted data from the database.

On a related note, please check your code for input sanitation. Just like
you encapsulate database access using a database "abstraction" library, you should encapsulate all data importing functionality from the user side of
your application and this encapsulation should perform strict sanity
checking on the data. That is, type and range should be enforced ("Is the
supposedly integer variable really containing a number?", "Is the select
input field actually returning a value from the available selection, or
something different?"), and data should be kept locally in your session
once it has been passed through the santiation once. Don't bounce data
using forms and hidden variables.

Kristian

Ok thank you for all the replies. Coming back to this topic after a little
break and I am confused. I have read your postings and am now not sure what
I should be doing. Now I originally was reading "PHP and MYSQL Web
development" by Welling and Thomas and they introduced to me sTripslashed
etc. They seem to encourage their use and I thought it looked great.

However on posting here I'm very confused. I get the impression that the way
I use this stuff should depend on what I am doing with a particular field.
But in practise this does not help.
The coding I am doing does not leave me as much time as I would like to have
the ultimate security. What I therefore need is a method that I can safely
apply to each field as the most secure measure. I kind of brute force
security approach so that if under pressure another person comes along at a
later time and adds a new field, there is a generic security filtering
applied to each field to prevent any attacks.

What is the best way of achieving this type of security. Is there not just
some standard agreed recommended practise?

Kind regards

Dave

[Dave Smithz]

"Dave Smithz" <SPAM FREE WORLD> wrote in message
news:42******@news1.homechoice.co.uk...

Ok thank you for all the replies. Coming back to this topic after a little
break and I am confused. I have read your postings and am now not sure what I should be doing. Now I originally was reading "PHP and MYSQL Web
development" by Welling and Thomas and they introduced to me sTripslashed
etc. They seem to encourage their use and I thought it looked great.

However on posting here I'm very confused. I get the impression that the way I use this stuff should depend on what I am doing with a particular field.
But in practise this does not help.
The coding I am doing does not leave me as much time as I would like to have the ultimate security. What I therefore need is a method that I can safely
apply to each field as the most secure measure. I kind of brute force
security approach so that if under pressure another person comes along at a later time and adds a new field, there is a generic security filtering
applied to each field to prevent any attacks.

What is the best way of achieving this type of security. Is there not just
some standard agreed recommended practise?

Hmmm, actually a guess the FAQ is the above really. After reading it a few
times its starting to make sense.

[Martin Lucas-Smith]

Dave wrote:

On a registration form, whenever users names have an apostrophe in them it
causes problems as they do not get added to the DB correctly for reasons
that immediately become apparent. What is the best way of introducing this practise across all of the code for
this project. Check if the magic quotes directive is enabled, and if so do nothing, but if
not, manually run the addslashes function and wherever data is retrieved
from the database go in and add a stripslashes.

Perhaps I'm misunderstanding the question, but isn't this merely a case of
adding

php_value magic_quotes_gpc true

(or whatever the exact syntax is here)
in your .htaccess or httpd.conf file, which should force magic quotes in
your PHP environment to be ON?

It sounds as if the code you are using assumes that magic_quotes IS set to
being on, such that all incoming data. (Doesn't sound like entirely top
quality code to me..)
Martin