CODEWITHSUNDEEP
phpsqlcode-injection
Anna Riekic
Anna Riekic

Reputation: 738

SQL injection: Is this line safe?

I have this line:

$sql = "UPDATE votes SET up=up+1 WHERE id='{$p}'";

Now from what I've read one way of sql injection is caused by not "closing" sql queries properly which allows hackers to add additional info.

So my question is, is that line safe as to me the up=up+1 has not been "closed" but if I set it like this up='up+1' which to me makes it "closed" it does not work.

row up type is int(11) if that makes any difference.

Update:

$p is sanitized with a function

function sanitize($foo) { 
if(get_magic_quotes_gpc() == true) { 
 $foo = stripslashes($foo); 
} 
return mysql_real_escape_string(htmlspecialchars($foo)); 
}

Upvotes: 1

Views: 125

Answers (2)

Jonathan Hall
Jonathan Hall

Reputation: 79516

It is not safe, unless $p is properly escaped. Otherwise imagine...

$p = "foo'; DROP TABLE votes; SELECT '1"

Then you'd end up executing something like:

UPDATE votes SET up=up+1 WHERE id='foo'; DROP TABLE votes; SELECT '1';

That wouldn't be very pretty...

Upvotes: 1

Michael Berkowski
Michael Berkowski

Reputation: 270599

The up=up+1 is not vulnerable because it does not accept user input via a variable. It is merely incrementing an INT value already in your database.

The WHERE id='{$p}' may be vulnerable, however, if the variable $p is not properly filtered and escaped, but as you added above, it has been sanitized by a function. Hopefully the sanitize function goes as far as checking the appropriate type of the variable (be it an int or string or whatever) and checks the appropriate bounds for it.

As always, the safest method is to make use of prepared statements and parameterized queries rather than pass any variables directly into a SQL string. Most of the database APIs available in PHP for the various RDBMS' support prepared statements. (Notably not the mysql_*() functions, however).

Upvotes: 4

Related Questions