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?

Answer 1

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.

Answer 2

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.

Answer 3

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 a function called 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.