Is this query safe from sql injection?

Question

The script is in PHP and as DB I use MySQL. Here is the script itself.

$unsafe_variable = $_GET["user-input"];
$sql=sprintf("INSERT INTO table (column) VALUES('%s')",$unsafe_variable);
mysql_query($sql);

Some people say that if user assigns ;DROP TABLE blah; string to the variable $unsafe_variable it deletes the table.
But I tried this example - http://localhost/test.php?user-input=DROP%20TABLE%20my_table and it didn't delete the table but instead inserted a new row (;DROP TABLE blah;) in the table.
Could anybody explain me how it is possible to attack this script with sql injections?
Thanks a lot.

Answer 1

mysql_query() doesn't allow the execution of multiple queries in one function. So you can't INSERT and then DROP the table. But you shouldn't rely on this as 'security'. Use parametrized queries instead. Check out PHP's PDO library.

However, they could change just about anything else, like possibly SELECTing a password field from another table as a subquery to place into that table so they can view the hash.

Answer 2

Nope, sprintf doesn't escape the content use:

$unsafe_variable = mysql_real_escape_string($_GET["user-input"]);
$sql=sprintf("INSERT INTO table (column) VALUES('%s')",$unsafe_variable);
mysql_query($sql);

Answer 3

mysql_real_escape_string($unsafe_variable)

Answer 4

While mysql_query only allows one query to execute, in general this query is not safe. An example of a dangerous input that would exploit your query is:

'); DROP TABLE my_table; --

The '); at the start will close your query and insert an empty value, but will allow for additional queries to be executed following the INSERT. Then after dropping a table, the -- at the end will mark everything else following (ie. the rest of your query) as a comment.

In order to safely prepare input for use in a query, use mysql_real_escape_string.

Answer 5

Some people say that if user assigns ;DROP TABLE blah; string to the variable $unsafe_variable it deletes the table.

Patently that's not the case - but if you don't understand why, then you can't tell if your code is safe. Are you going to post every line here to check if its safe?

Without going into a long explanation about what the code above is doing and how to compromise it (SQL injection is already very well documented elsewhere - try Google for a start) you should ALWAYS ensure that any data leaving your PHP code is in the correct representation for where it is going.

For a MySQL database that means either:

1) use the output of mysql_real_escape_string (and make sure you pass the right resource handle)

or

2) use parameter binding.

A proper discussion of code injection attacks could easily fill several hundred pages - a bit much to answer in a S.O. query.

C.

Answer 6

I think the example you would need to try is http://localhost/test.php?user-input=';DROP%20TABLE%20my_table'

the '); closes the values('%s segment, and then issues a new command, drop table...

Answer 7

The ONLY way you should be handling unsafe variables is with bind parameters.

Please read this page on how to prevent SQL injection from bobby-tables.com.

Answer 8

That particular injection wouldn't work since PHP's mysql_query function only allows one query per call. However, the following may work if column has a primary or unique key:

$unsafe_variable = "admin') ON DUPLICATE KEY UPDATE password=MD5(CONCAT('knownsalt', 'newpassword'))#";

Better to use the long-winded mysql_real_escape_string function:

$sql=sprintf("INSERT INTO table (column) VALUES(%s)",
             mysql_real_escape_string($unsafe_variable));
mysql_query($sql);