I've seen dozens of PHP snippets that go like this:
function DB_Quote($string)
{
if (get_magic_quotes_gpc() == true)
{
$string = stripslashes($string);
}
return mysql_real_escape_string($string);
}
What happens if I call DB_Quote("the (\\) character is cool");? (Thanks jspcal!)
Aren't we supposed to strip slashes only when get_magic_quotes_gpc() == true and the value originated from $_GET, $_POST or $_COOKIE superglobals?
Yeah, I've seen dozens of PHP snippets like that, too. It's a bit sad.
Magic quotes are an input issue. It has to be fixed at the input stage, by iterating the GET/POST/COOKIES arrays and removing the slashes, if you need your app to run on servers using the foul archaic wrongness that is magic_quotes_gpc. The simple alternative is to detect the magic quotes option and die with a “your server sucks” error when set.
mysql_real_escape_string is an output issue. It needs to be run on the way out of the script, on content heading to the database, if you're not using parameterised queries (which you should definitely consider).
These are two separate unrelated stages in the program. You can't put them in the same function, tempting though it may be to try to encapsulate all your string processing into one box.
Aren't we supposed to strip slashes only when [...] the value originated from $_GET, $_POST or $_COOKIE superglobals?
Yes, exactly. Which is why the snippet you quoted is indeed harmful. Because tracking the origin of a string is impractical (especially as you might combine strings from different sources, one of which is slashed and the other not), you can't do it in one function. It has to be two separate string handling functions called at the appropriate time.
Step one is to turn off magic quotes altogether and issue large unignorable warnings if it is on.
If this isn't an option to you, then in my opinion, the best way to go is to remove all magic quotes all the time.
// in an include used on every page load:
if (get_magic_quotes_gpc()) {
foreach (array('_GET', '_POST', '_COOKIE', '_REQUEST') as $src) {
foreach ($$src as $key => $val) {
$$src[$key] = stripslashes($val);
}
}
}
A small performance hit, but will give you a LOT more ease of mind when working with your variables from then on.
yeah as dav said, up to the script to only use stripslashes on form input...
if you call stripslashes with the string "O'Reilly", the string won't be changed. if you call stripslashes with a string like: "the backslash (\) character is cool", the result will replace the escape sequence \) with ). so, using that function on all db values could subtly break things, but it might not be noticed.
like many apps, you could also just not support magic quotes mode. check for it and print an error if it's on. it's been off by default for years and it's not recommended.
You could try doing the following on $_GET, $_POST or $_COOKIE before doing anything else with them:
<?php
if (get_magic_quotes_gpc ())
{
$_POST = array_map ('stripslashes', $_POST);
}
?>
Note, this will only work if your input array is "flat" (doesn't contain sub-arrays). If you expect sub-arrays in the input then you'll need to write your own callback to handle it. Something like
<?php
function slashField ($input)
{
if (is_array ($input))
{
foreach ($input as $key => $row)
{
$input [$key] = slashField ($row);
}
}
else
{
$input = stripslashes ($input);
}
return ($input);
}
if (get_magic_quotes_gpc ())
{
$_POST = array_map ('slashfield', $_POST);
}
?>
Number 6 is the right idea but doesn't work, because until PHP 5.4 $$src[$key] uses variable variable syntax on $src[$key] which is interpreted as $src[0], creating a variable $_ which is useless. Just use & (the reference operator) on $val and $val = stripslashes($val). In addition, otherwise from PHP 5.4 on, you probably get an error because PHP won't just cast the _ to 0
