MySQL Injection Confusion, How Does it Actually Protect?

Question

I've been reading about MySQL Injection as I've just started adding prepared statements to my site. I wanted to know a bit more about how it actually protects and after reading some, I don't think I've done it correctly.

I was reading from here: http://www.tizag.com/mysqlTutorial/mysql-php-sql-injection.php

I really don't understand how it can be 'injected' or how prepared statements get around it. Particularly this bit:

Normal: SELECT * FROM customers WHERE username = 'timmy'
Injection: SELECT * FROM customers WHERE username = '' OR 1''

When using SELECT, the things I use for WHERE are only ID or username. They can't change their ID and the username is validated when they sign up by this:

function protect($string) {
global $con;
return mysqli_real_escape_string($con,strip_tags(addslashes($string)));
}

So is this "protect()" preventing MySQL injection?

If so, what are the uses for prepared statements? Is this protection correct?

Instead of

$car = 'nissan';  
$update_car = mysqli_query($con,"UPDATE stats SET stats.car = $car WHERE stats.id = 4");

I put:

$car = 'nissan';  
$query = "UPDATE stats SET stats.car = $car WHERE stats.id = 4";  
$update_car = mysqli_query($con,$query);

If that is correct, I don't understand how that does anything besides just adding another line of code?

EDIT

First prepared statement, any good?

$getusername = mysqli_prepare($con, "SELECT users WHERE users.username = '",mysqli_real_escape_string($username),"'");

Answer 1

Also note that mysqli_real_escape_string is to prevent injection with strings. If you have a (for example) integer field then escaping the passed variable to cope with a quote doesn't really help as injection doesn't require the quote to work.

For example:-

$some_field_expected_to_be_int = '1 OR 1=1';
$query = "SELECT * FROM users WHERE id = ".mysqli_real_escape_string($con, $some_field_expected_to_be_int);

would still give a piece of SQL reading:-

SELECT * FROM users WHERE id = 1 OR 1=1

and return everything.

In such a case you should make sure it is an integer:-

$some_field_expected_to_be_int = '1 OR 1=1';
$query = "SELECT * FROM users WHERE id = ".(int)$some_field_expected_to_be_int;

Personally both prepared statements and escaping fields have their places. Prepared statements make it harder to forget to escape items and with many databases they give a performance advantage, but the mysqli placeholders are not readable, getting a statement out for debugging is not workable and it becomes a nightmare when used for heavily dynamic sql. But escaping leaves you with the risk you will forget somewhere and the code can look messy.

EDIT - to add a bit on prepared statements.

We use a couple of in house database classes which separate things a bit from mysql / mysqli / pdo / sql server, but for a basic example of using prepare and a bound parameter:-

<?php
    if($stmt = $this->db->prepare("SELECT users WHERE users.username = ? "))
    {
        $stmt->bind_param('s', $username);
        $stmt->execute();
    }

?>

In this the SQL is not built up using the variable (which could have been manipulated), rather the variable is passed as a parameter.

Answer 2

Firstly, let's look at your attempt at escaping:

function protect($string) {
global $con;
return mysqli_real_escape_string($con,strip_tags(addslashes($string)));
}

There are 2 mistakes here: Firstly, addslashes is doing the same job as mysqli_real_escape_string (but doing it badly), so remove that.

Secondly, you should always sanitise for the context you're working in. So, when generating SQL, you sanitise for SQL; when generating HTML, sanitise for HTML. So putting strip_tags in here doesn't make sense, because it's to do with HTML, but this is apparently an SQL escaping function. Do that separately, when you're preparing output in your templates.

In short, just mysqli_real_escape_string on its own would be better than what you have here.

Next, let's look at parameterising the query:

$car = 'nissan';  
$query = "UPDATE stats SET stats.car = '$car' WHERE stats.id = 4";  
$update_car = mysqli_query($con,$query);

This statement isn't prepared or parameterised - as far as the database is concerned, it's still just a string of SQL. If $car is actually set from user input (e.g. $car = $_GET['car'];), then you can insert any piece of SQL in the middle of the query:

$car = "nissan'; DROP TABLE stats; --";  
$query = "UPDATE stats SET stats.car = '$car' WHERE stats.id = 4";  
$update_car = mysqli_query($con,$query);
// MySQL will now drop your table; oops!
// UPDATE stats SET stats.car = 'nissan'; DROP TABLE stats; --' WHERE stats.id = 4

Adding $car = mysqli_real_escape_string($car); will correctly escape the ' in the input, stopping it ending the string and starting a new SQL statement.

Parameterised queries avoid the problem a different way: they tell the database which parts of the query are supposed to be SQL, and which are just data provided by the user. It looks like this:

$car = "nissan'; DROP TABLE stats; --";  
$query_with_placeholder = "UPDATE stats SET stats.car = ? WHERE stats.id = 4";  
// Note the ?, without quotes, represents somewhere for data to be put by MySQL
$prepared_statement = mysqli_prepare($con, $query_with_placeholder);
// Now we tell MySQL what to put in the placeholder
mysqli_stmt_bind_param($prepared_statement, 's', $car);
// And execute it
mysqli_stmt_execute($prepared_statement);

Because $car is never actually inserted into the SQL, just passed as a separate parameter, there is no way of it injecting anything nasty, and we don't need to do any additional escaping.

Answer 3

Escaping the data and using prepared queries both protect against SQL injection. But escaping is more error-prone, because it's easy to forget to call it in all the places you need it. If you use prepared statements, the protection happens automatically.

There's no significant difference between

$update_car = mysqli_query($con,"UPDATE stats SET stats.car = $car WHERE stats.id = 4");

and

$query = "UPDATE stats SET stats.car = $car WHERE stats.id = 4";  
$update_car = mysqli_query($con,$query);

This is just a stylistic choice. The second version can be useful when you want to insert debugging statements, e.g.

$query = "UPDATE stats SET stats.car = $car WHERE stats.id = 4";  
echo $query;
$update_car = mysqli_query($con,$query);

Answer 4

Popular practice is to both sanitize input and use prepared statements with your queries whenever you have a possible user input variable.

Sanitation should also prevent users from overflowing your variables and upload large quantities of data to your application.

Preparing statements is your second measurement of security, it prevents mistakes you might make later, like forgetting to escape input or not realising what is actually user input next time you refactor your application.

Any time you don't properly guard against injection, you open your project up to users deleting data, modifying data or uploading malicious code to your application. (Do you really want someone to upload javascript in a textfield?)