0

Possible Duplicate:
How to prevent SQL injection in PHP?

I want to know if my code has hacks like SQLI

function insertRow($table,$fields,$values)
    {
        if(count($fields) != count($values))
        {
            echo "fields and values must be the same count";
            return null;
        }
        $query = "INSERT INTO ".$table." SET ";
        foreach($fields as $key => $field)
        {
            $query = $query. "" . $field . " = '" . htmlspecialchars($values[$key], ENT_QUOTES) . "', ";
        }
        $query = substr($query,0,-2);

        if (!mysql_query($query, $this->con))
        {
            echo "Error : " . mysql_error($this->con)."<br />";
            return false;
        }
        return true;
    }

I use htmlspecialchars and I want to know if it is ok

Edit :

$fields = array("a","b","c");
$values = array($_POST["a"],$_POST["b"],$_POST["c"]);
$a = $dbc->insertRow("tbl_synagoge",$fields,$values);
Community
  • 1
  • 1
Aviv Cohen
  • 108
  • 6
  • 5
    [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://j.mp/PoWehJ). – jeremy Dec 29 '12 at 02:55
  • 4
    `htmlspecialchars()` is for sanitizing _output_ data to the browser, not for sanitizing input. We would need to see how this function is called to know if it is safe (though I suspect it is not), what's important is how `$fields, $values` get populated. – Michael Berkowski Dec 29 '12 at 02:56
  • 3
    See http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php – Michael Berkowski Dec 29 '12 at 02:56
  • Please go learn about PHP's multiple ways of preventing SQL injection, preferably the PDO module. See http://bobby-tables.com/php.html – Andy Lester Dec 29 '12 at 03:04
  • @AndyLester, I'm reconsidering recommendation of PDO since I found out PDO_mysql enables multi-query *by default*. It's exactly what allows the "Bobby Tables" vulnerability. – Bill Karwin Dec 29 '12 at 03:09
  • 1
    More resources: https://www.owasp.org/index.php/SQL_Injection, and http://www.slideshare.net/billkarwin/sql-injection-myths-and-fallacies – Bill Karwin Dec 29 '12 at 03:10
  • “if my code has hacks like SQLI”? It should be more like “if my code is vulnerable to SQLI”. Your code is the victim and not the actor. – Gumbo Dec 29 '12 at 09:47

1 Answers1

3

Instead of htmlspecialchars() use mysql_real_escape_string().

This will make your code more secure.

However you might want to retire all your mysql_* functions as they are deprecated. You can get started right here: MySQLi

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Ibu
  • 42,752
  • 13
  • 76
  • 103