views:

52

answers:

2

I'm currently developing a database class for a project I've been working on. I would like to know if the following function has any holes or errors in it that would allow for someone to use MySQL injection.

public function sql_validate($var)
{
    if (is_null($var))
    {
        return NULL;
    }
    else if (is_string($var))
    {
        return "'" . $this->sql_escape($var) . "'";
    }
    else if (is_bool($var))
    {
        return intval($var);
    }
    else
    {
        return $var;
    }
}

Here's the function sql_escape which is called for strings.

private function sql_escape($string)
{
    if (!$this->db_connection)
    {
        return @mysql_real_escape_string($string);
    }

    return @mysql_real_escape_string($string, $this->db_connection);
}
+5  A: 

mysql_real_escape_string uses whatever escaping mode is best, therefore it is fairly secure.

However, may I suggest an alternative. Why create a database class when there are several out there which utilize a much more secure method - Prepared statements. In a nutshell these work by separating SQL logic from user inputted data.

I would suggest using the PDO CLASS.

Also, you should really avoid using error suppression @ in PHP. As this tends to suppress errors which you might not even expect.

The following article is a fairly straightforward introduction to PDO.

Russell Dias
+2  A: 

Looks like you're using dynamic queries.

Consider using prepared statements instead.

Also see http://stackoverflow.com/questions/3553120/what-are-prepared-statements-how-are-they-different-from-dynamic-sql

Andrew67