PHP / MySQLi: How to prevent SQL injection on INSERT (code partially working)

Question

II would like to store two values (an email and a password) in a MySQL db using PHP. The input is passed to the PHP page via Ajax in jQuery (through an onclick event on a website).

Now I want to protect this PHP query against SQL injection - I am talking about a standard website so I don't want to overdo it but I think some standard protection can't be bad.

Below is what I had before just to ensure that the general procedure is working which it is. I then tried to increase security by using password encryption and statements but I am not sure if I did this the right way + I don't know if and how this needs to be applied to the Select part as well (where I am checking if the email already exists).

I've seen plenty of pages on this topic but as a beginner that's exactly my problem here. Can someone tell with this as an example what should be changed or added here and maybe provide some short explanations ? I would like to keep using MySQLi if possible.

Old PHP:

$conn = new mysqli($servername, $username, $password, $dbname);
$conn->set_charset("utf8");
if($conn->connect_error){
    die("Connection failed: " . $conn->connect_error);
}
$email = $_POST["email"];
$pw = $_POST["pw"]; 

$sql = "SELECT email FROM Users WHERE email = '" . $email . "'";
$query = $conn->query($sql);
if(mysqli_num_rows($query) > 0){
    echo "Record already exists";
}else{
    $sql = "INSERT INTO Users (email, pw) VALUES ('" . $email . "', '" . $pw . "')";
    if($conn->query($sql)){
        echo "Update successful";
    }else{
        echo "Update failed";
    };
}
$conn->close();

New PHP:

$conn = new mysqli($servername, $username, $password, $dbname);
$conn->set_charset("utf8");
if($conn->connect_error){
    die("Connection failed: " . $conn->connect_error);
}
$email = $_POST["email"];
$pw = password_hash($_POST["pw"], PASSWORD_BCRYPT); 

$sql = "SELECT email FROM Users WHERE email = '" . $email . "'";
$query = $conn->query($sql);
if(mysqli_num_rows($query) > 0){
    echo "Record already exists";
}else{
    $sql = $conn->prepare("INSERT INTO Users (email, pw) VALUES ('" . $email . "', '" . $pw . "')");
    $sql->bind_param('s', $name);
    $sql->execute();
    $result = $sql->get_result();
    if($result){
        echo "Update successful";
    }else{
        echo "Update failed";
    };
}
$conn->close();

Look into prepared statements: MySQLI http://php.net/manual/en/mysqli.prepare.php / PDO http://php.net/manual/en/pdo.prepared-statements.php — RuubW, Jun 22 '15 at 07:14

score 2 · Accepted Answer · answered Jun 22 '15 at 07:24

2

In the New PHP code snippet, you are still vulnerable to injections.
You are using a prepared statement in the insert part, but you are not actually using the preparations strengths correctly.

When creating a prepared statement, you create a query in which you add placeholders instead of the raw values:

$stmt = $conn->prepare("INSERT INTO Users (email, pw) VALUES (?, ?)");

The question marks are the placeholders and are later replaced by using the bind_param method:

$stmt->bind_param('ss', $email, $pw);

The ss part of the bind call tells the mysql db that its two strings that are passed to the database (s for string, i for int etc).
You are binding a param ($name) but it has no placeholder nor any type of reference in the query..?

Your select statement on the other hand is still unsafe and open to vulnerabilities.
I would probably use a prepared statement there to, just as with the insert part.

You always want to make sure that input from the user is "safe" for the database, if you concat a query string and add user input into it, the database will not escape the strings, it will just run it.

Only use standard query method calls when you write the full query yourself, without any input params, and especially no input params that the user passed!

answered Jun 22 '15 at 07:24

Jite

5,761
2
23
37

Thanks a lot for this and sorry for the late response. This is awesome and the explanations here are really helpful ! Is there a way I can send you the updated query once I completed the above ? Just want to make sure it is ok then since this is the first time I am writing something like this. – keewee279 Jun 22 '15 at 09:01
Update: I revised the query in the post trying to implement your suggestions. Could you or someone else let me know if this is correct now according to your approach ? – keewee279 Jun 22 '15 at 09:13
1

Looks a lot better. You could change your `mysqli_num_rows($result)` to use the OOP mysqli style(which you use in all the other code): `$stmt->num_rows`. You could also throw in a regex check on the email to validate it (so that it is actually an email address), but what you have now should work (have not ran code, so there might be syntax errors that i cant see in the code snippet, but the general idea is correct, hehe). – Jite Jun 22 '15 at 09:19
Thanks so much ! I am trimming and validating the email and the password in jQuery before passing it to the PHP page. Regarding your first line you mean to replace mysqli_num_rows($result) by $stmt->num_rows($result) ? – keewee279 Jun 22 '15 at 09:21
Update: I checked this but it now. The insert works and also the check for existing records but for new inserts it now always returns "Update failed". Any thought what could cause this ? – keewee279 Jun 22 '15 at 09:33
1

No need, the statement already know about the result, so all you need to do is check the `num_rows` property of the statement object. Data from javascript can always be modified by the user, thats why its so important to validate all input on the server side (php). Now, the email address wont be vulnerable, due to the prepared statements, but it could potentially be something else than a email address. – Jite Jun 22 '15 at 09:35
Thanks - just updated my previous comment. Since I validate the email in jQuery the value that I pass is always an email address. Do I have to check this in PHP again ? – keewee279 Jun 22 '15 at 09:35
1

Instead of checking `$result`, check if there is any error in the statement: `if($stmt->errno !== 0) { echo "Update failed: " . $stmt->error; }` Should show you the error. – Jite Jun 22 '15 at 09:37
1

Well, you don't HAVE to validate it again, but it would be good to do if you wish to make sure that it actually is an email address, as it could have been tampered with by the user. – Jite Jun 22 '15 at 09:39
Awesome - it works perfect now ! Thanks so much for that. Regarding the email I check a number of things in jQuery (like spaces, no @ sign, double @ sign, @ sign at the wrong position, length etc.). What else would you recommend here in PHP ? – keewee279 Jun 22 '15 at 09:41
Happy to help. For email validation, id do a search here on stack overflow or google. Good luck. :) – Jite Jun 22 '15 at 09:44
Will do. Thanks again - you really helped me to understand this better. :) – keewee279 Jun 22 '15 at 09:45

score 2 · Answer 2 · answered Jun 22 '15 at 08:15

2

$query = $mysqli->prepare("INSERT INTO `table` (`col1`, `col2`) VALUES (?, ?)");
$col1 = 100;
$col2 = 14;
$query->bind_param('ii', $col1, $col2);
$query->execute();
$query->close();

answered Jun 22 '15 at 08:15

Abdullah Al Shakib

2,034
2
15
16

Thanks for this as well and sorry for the late response ! – keewee279 Jun 22 '15 at 09:01

PHP / MySQLi: How to prevent SQL injection on INSERT (code partially working)

2 Answers2

Linked