Is this hacker prevention elegant enough?

[webref.eu]

Hi All

 

I have some code that checks whether my ProductId, which is passed in a query string, is just a number, to prevent a hacker attempting to replace it with something malicious.  My question is, is my code elegant enough or can you suggest improvements?  Thanks. 

 

//Protection from hackers.  Check ProductId is just a number
$TestForNumber = is_numeric($ProductId);
If ($TestForNumber == 0) {
echo "Sorry, the Product Id tried is not allowed.";
exit();
}

 

Rgds

 

 

[wildteen88]

Yes, if your product id is only supposed to contain a number then check it is a number before using it in your query.

 

Example url:

yoursite.com/product.php?pid=12324

 

This is the code I'd use to check to see if pid only contains a number

if(isset($_GET['pid']) && is_numeric($_GET['pid']))
{
    $product_id = $_GET['pid'];

    // rest of your code here
}
else
   die('Product ID code invalid');

[xylex]

Careful how you use that.  Hex values, like 0x61646D696E, pass the is_numeric() check, but become a string value if entered unquoted into a MySQL db query.  You might be better off using filter_var() or casting to an int if a number is what you always want.

 

[laffin]

I use something like this

$pid=$_GET['pid']>0?$_GET['pid']:0;

 

just because rarely do u need a negative values, and setting up a failure conditional for yer messages

 

if(!$pid) die('Invalid ID');

 

later in yer code.

But I think u should think how each variable needs to be validated. some will have different requirements than others good luck

[Mchl]

Or use casting to (int)