Antidote for magic_quotes_gpc()?

Question

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?

Statistical

Yes, but there's no way to track the origin of a value for a function that's meant to be generic. So the onus is on the programmer to only call the snippet for things that are coming from those superglobals. – Harmony

actually - with your example of "O'Reilly" - you'll get a properly escaped string "O\'Reilly"... the problem occurs when you want to insert a string like "O\'Reilly" (as an example of a properly escaped string which you want to display later) - because DB_Quote will simply return "O\'Reilly" instead of "O\\\'Reilly", and subsequently selecting this from the database will give you "O'Reilly" – Tortricid

@HorusKol: Yeah, you're right. Sorry for that. – Statistical

Answer 1

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.

Timisoara

+1 "The simple alternative is to detect the magic quotes option and die with a “your server sucks” error when set." – Frasch

Answer 2

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.

Complaisance

agreed - better to turn off... or change to another host – Tortricid

+1, I use similar code in all of my projects too. Magic quotes are the most vicious malice of PHP known to date. The intent was good, but the realization is vicious and error-prone. Disable is good, and in software (written to be distributed on any PHP thing) you just have to use this workaround (the code of this answer, executed as early as possible). – Ostrich

@nickf: What about $_REQUEST, should I also stripslash it or are the changes reflected automatically? – Statistical

I'm used to adding such kind of snippet to all my PHP projects now. Magic_quotes is a evil function and I'm glad they will remove it in PHP6 (Or us developers will get a nightmare) – Metamathematics

@nickf: Superglobals (like $_POST) can not be used as variable variables within functions. This code must be outside a function in order to work. – Burschenschaft

Answer 3

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.

Seasonseasonable

It does change it: DB_Quote("O'Reilly") == O\'Reilly. – Complaisance

stripslashes doesn't change it, but mysql_real_escape does – Seasonseasonable

Answer 4

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);
}
?>

Althing

Answer 5

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

Pyromania