PHP magic_quotes_gpc vulnerability

Question

I've been assigned to one of my company's legacy webapps, and after a day or two of poking around the source, I've found an SQL injection vector similar to the following:

mysql_query("SELECT * FROM foo WHERE bar='" . $_GET['baz'] . "'");

I've tried to perform an SQL injection test against this, but it fails, due to PHP's magic_quotes_gpc module being switched on.

I know magic_quotes_gpc is dirty, but we have hundreds - if not thousands - of lines of code similar to the one above. We simply can't afford to switch magic_quotes_gpc off, as this would leave code like this wide open to attack.

I'd like to know how 'exploitable' the code above is, and whether we should fix it immediately, or include the task of fixing it in with our other refactoring tasks.

Answer 1

The usual way to transition sites away from magic_quotes_gpc is to add a wrapper function:

function m($s) {
    if (get_magic_quotes_gpc())
        $s= stripslashes($s);
    return mysql_real_escape_string($s);
}

mysql_query("SELECT * FROM foo WHERE bar='".m($_GET['baz'])."'");

This will fix the problem of addslashes not being character-set-aware that can cause it to be vulnerable in some cases, and will generally make the code continue to ‘work’ as before.

However in the long term relying on input-escaping is unsustainable, as it will multiply slashes into input strings you are not inserting into the database, and fail to escape strings you're inserting into the database from other sources. This is the real reason magic_quotes_gpc is the wrong thing: it's applying an output-stage encoding to the input stage.

So add the wrapper function and then slowly go through updating all the SQL interpolations to use it. When you've got them all you can turn the magic quotes off.

Answer 2

As long as magic quotes are on, and you don't use some special character encoding which could slip things through it, you should be fine -- so to speak. Problem is, when for whatever reason magic quotes are not active (server change, configuration change) you'll have a lot of holes to fix.

Answer 3

I would add a line at the beginning that makes sure magic_quotes are enabled, also if they are disabled in the server configuration. Then you'll be more or less safe.