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 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:
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.