Trying to prevent sql injection - Grendel

[Jax2]

I am trying to prevent SQL injections and I thought I had my problems solved, but when using Grendel to test my site, it is telling me I have an issue in two of my pages.

 

The first is browse.php and the problem is with the $cat variable. I am using a cleaning function:

 

function anti_injection($sql) 
{
   $sql = preg_replace(sql_regcase("/(from|select|insert|delete|where|drop table|show tables|#|\*|--|\\\\)/"),"",$sql);
   $sql = trim($sql);
   $sql = strip_tags($sql);
   $sql = addslashes($sql);
   return $sql;
}

 

Now, in the page in question, I have it set like this:

 

<?php
$cat=anti_injection($_GET['cat']);
$rowsperpage = 10; // how many items per page
$range = 10;// how many pages to show in page link
$sql="SELECT * FROM ".$prefix."categories where ID=$cat";
$result=mysql_query($sql, $db);
while ($row=mysql_fetch_array($result))
{
$catname=$row['category_name']; 
}
?>

 

Which I thought would have me covered, as that is the ONLY location where I actually call on $_GET['cat']

 

Yet, as I've said, Grendel is warning me that it's vulnerable to an SQL injection and I've got no idea how or why.

 

Here is what Grendel returned:

 

When a single quote (') was appended to the parameters listed below, a SQL error message was returned. This could indicate a SQL injection vulnerability.

 

URL: http://www.XXXXXX.com:80/XXX/XXX/browse.php

Parameter name: cat

Platform: MySQL

[Alex]

To prevent mysql injections you should pass all data through mysql_real_escape_string before it's inserted into the query.

[Jax2]

Okay, I have tried changing my anti-injection function to this:

 

function anti_injection($sql) 
{
   $sql = preg_replace(sql_regcase("/(from|select|insert|delete|where|drop table|show tables|#|\*|--|\\\\)/"),"",$sql);
   $sql = trim($sql);
   $sql = strip_tags($sql);
   $sql = addslashes($sql);
   return mysql_real_escape_string($sql);
}

 

And I ran another grendel scan. It returned the same issue.

[DavidAM]

I don't think you want to addslashes() if you are going to use mysql_real_escape_string().

[Jax2]

I am not a security guru, and I'm far from knowing a lot about it, so bear with me please.

 

I got rid of the function call completely and tried simply using:

 

$cat=mysql_real_escape_string($_GET['cat']);

 

just to see if that would solve the problem and it's not. I'm still getting the warning. I've also looked into a few different online sql injection testing websites and they've all confirmed it's open to sql injection.

 

This is the ONLY variable I call using either a $_POST or $_GET call, so it's the ONLY variable a person could mess with, so the problem has to be there on the page. It's driving me nuts.

 

[Jax2]

I found a solution while looking for sanitizing functions online. I found this one, and it works perfectly. No more sql injection errors anywhere on my site WOO!

 

function sanitize($input) {
    if (is_array($input)) {
        foreach($input as $var=>$val) {
            $output[$var] = sanitize($val);
        }
    }
    else {
        if (get_magic_quotes_gpc()) {
            $input = stripslashes($input);
        }
        $input  = cleanInput($input);
        $output = mysql_real_escape_string($input);
    }
    return $output;
}