Upgrading PHP - Need to remove get_magic_quotes_gpc

Question

We're upgrading from php 5.3 to 5.4, which is not backwards compatible for 'get_magic_quotes_gpc'. I understand the code will still work, sort of, but just bring back a FALSE each time.

However, I think the time has come to scrub this from our code.

Here's a typical example:

     $product_id = "0";
        if (isset($HTTP_GET_VARS["id"])) {
          $rid = (get_magic_quotes_gpc()) ? $HTTP_GET_VARS["id"] : addslashes($HTTP_GET_VARS["id"]);
        }

       //then $product_id gets used all over the place in many different queries

I've been researching how to fix this and this is what I came up with:

    $rid = "0";
    if (isset($HTTP_GET_VARS["id"])) {
    $rid = addslashes($HTTP_GET_VARS["id"]);
    }

I'm a little over my head here. I know this all has to do with SQL injection and such. Is my solution an reasonable/acceptable one?

Thanks in advance.

<<<< EDIT - ADDITIONAL INFORMATION >>>>

Thanks for the replies. Actually, we did a bunch of conversion to PDO about 18 mos ago (mostly due to this type of advice on stackoverflow :)

So, I may have some reduntant, pointless code going on. Here's the full picture of what is happening below the code I posted above that gets the variable from the URL.

You'll see, there is the (get_magic_quuotes_gpc) that used to be there, now commented out and replaced by the (addslashes). But that variable is passed on to a PDO query.

$product_id = "0";
if (isset($HTTP_GET_VARS["id"])) {
  //$product_id = (get_magic_quotes_gpc()) ? $HTTP_GET_VARS["id"] : addslashes($HTTP_GET_VARS["id"]);
  $product_id = addslashes($HTTP_GET_VARS["id"]);
}

// NEW QUERIES - BEG xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

try {  
    # MySQL with PDO_MYSQL  
    $pdo = new PDO("mysql:host=$hostname_db;dbname=$database_db", $username_db, $password_db);  
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

    // Query 1:  product details
    $stmt = $pdo->prepare('SELECT 
    [a bunch of stuff here, fields, joins, etc]
    WHERE product_id = ? ');  
    $stmt -> execute(array($rid));
    $row_count_resto_details = $stmt->rowCount();
    $row_resto_details = $stmt->fetch(PDO::FETCH_ASSOC);


}  
// Error message for pdo
catch(PDOException $e) {  
    echo $e->getMessage();  
}  


// END QUERY xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

Can I just get rid of all the stuff in the first 4-5 lines of code and just make it this:

$product_id = $HTTP_GET_VARS["id"];

no. it's not. addslashes is utterly USELESS for sql injection. don't try to recreate old/stupid/moronic things in PHP. magic_quotes were removed for a reason. update your code to modern standards, using mysqli or PDO, with prepared statements and placeholders. — Marc B, Aug 05 '15 at 18:52
Use PDO and you won't have this problem. It is better practice to use it compared to old methods and in most ways, quicker. — RhapX, Aug 05 '15 at 18:54
thanks both for answering. I've show some of my additional code, which actually is already a PDO query. Can you please take a look, I guess my question has changed a little. — Kevin, Aug 06 '15 at 01:40

score 5 · Accepted Answer · edited May 23 '17 at 11:57

No, it's not reasonable.

The problem is, the "magic quotes" feature was a bad idea in the first place, so trying to replicate it only means that you've relied on a broken solution until now, and your application is without a doubt vulnerable to SQL injection attacks.

"Why is magic quotes a broken solution?", you'd probably ask. And the answer is kind of hidden in the question itself - security and IT in general aren't and can't be magic. Whenever you see a solution that advertises itself or at least seems to work in a "magic" way, know that it is bad and you should never trust it.

What you need instead are context-aware solutions, and in the case of preventing SQL injections - that's parameterized queries. You should read this to learn more: How can I prevent SQL-injection in PHP?

I'd also urge you to upgrade straight to PHP 5.6, mostly for two reasons:

PHP 5.4 will reach its End Of Life in a month and that alone makes it a possible security risk.
You have no reason not to. Honestly, the upgrade path is a breeze and comes with very little backwards-compatibility concerns, if any at all. PHP is much more stable now, there are very few significant changes since 5.3 vs 5.4.

Update (to answer the extended question):

You not only can remove the addslashes() logic, but you MUST do that - if you leave it, it will add slashes to some of your input data and these slashes will be a part of the data itself.
What you want to do instead is to validate input data - check if it is in the proper format in the first place. For example, if you expect a numeric ID - check if it only contains digits before using it.

Also, $HTTP_GET_VARS has been deprecated since PHP 4.1 and you should be using $_GET instead.

thanks for the answer to my question. But could you please take a look at my edited post. As it turns out, we already converted to PDO. So, I'm trying to figure out if this $HTTP_GET_VARS with the magic/addslashes is just old legacy code and also what exactly should I be using to grab the variable from the URL? Is $id=$HTTP_GET_VARS enough? — Kevin, Aug 06 '15 at 01:42

Upgrading PHP - Need to remove get_magic_quotes_gpc

1 Answers1