tags:

views:

147

answers:

2

I have a PHP script with the following line:

$query = "SELECT * FROM products WHERE product_id='" . filter_var($_GET[id], FILTER_SANITIZE_NUMBER_INT) . "'";

Is this safe enough? How would you improve this code?

+2  A: 

It is safe for that case, but for a more general approach, I'd rather use mysql_real_escape_string in conjunction with type casting:

$query = "SELECT * FROM products WHERE product_id='" . (int)mysql_real_escape_string($_GET['id']) . "'";

In the worst case, that will result in a 0 and will escape all malicious input also. mysql_real_escape_string can be used on all kinds of data to make it safe for queries, which makes it the most versatile of all escape/sanitation functions.

Without going as far as using prepared statements, you can use sprintf to create your SQL and to handle the type casting automatically:

$query = sprintf("SELECT * FROM products WHERE product_id = '%d'", mysql_real_escape_string($_GET['id']));

See the sprintf entry from the PHP manual for the syntax.

It gets even simpler if you use array_map to escape all $_GET and $_POST variables, then you can use them as is:

$_GET = array_map('mysql_real_escape_string', $_GET);
$_POST = array_map('mysql_real_escape_string', $_POST);

$query = sprintf("SELECT * FROM products WHERE product_id = '%d'", $_GET['id']);
Tatu Ulmanen
+1, also there is better support for this since filter_var is only available on PHP 5.2 or higher.
ILMV
Thank you for the thorough answer.
Evenz495
+1  A: 

I usually just use intval:

$product_id = intval($_GET['id']);
$query = "SELECT * FROM products WHERE product_id='" . $product_id . "'";
Emil Vikström