Preventing SQL Injection in user registration routine

Question

At the moment is my code secure for SQL injections and so forth? I still need to hash passwords and make sure fields are valid and so forth.

<?php
try{
 $handler = new PDO('mysql:host=localhost;dbname=s','root', '*');
 $handler->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e){
 echo $e->getMessage();
 die();
}
$name = $_POST['name']; 
$username = $_POST['username']; 
$email = $_POST['email']; 
$password = $_POST['password']; 
$sql = "INSERT INTO userinfo (name ,username, email, password) VALUES (:name,:username,:email,:password)";
$query = $handler->prepare($sql);
$query->execute(array( 
 ':name' => $name,
 ':username' => $username,
 ':email' => $email,
 ':password' => $password
));
?>

• azngunit81: SQL attack has been over asked both here and on stackoverflow: stackoverflow.com/questions/60174/…, and stackoverflow.com/questions/60174/… both articles are applicable to what you are asking.
• azngunit81: possible duplicate of Is this code safe from SQL injection?
• 200_success: @azngunit81 Same topic, different code. I don't see how this is a duplicate.
• azngunit81: @200_success same PDO setup with similar code. Its an insert with PDO setup so the same response will be yield: the post needs to be sanitize, prepare statement is already use so that is fine but everything that is said in the articles mentioned is a repetition of similar response that would be drafted up - hence duplication of question.
• 200_success: @azngunit81 Remember that all aspects of the code are reviewable, not just the SQL injection issue that the OP raised. Questions submitted by two different users are almost never duplicates of each other.

Answer 1

It is safe.

You can improve your code like this:

• no need to use closing ?> in case that you are not outputting any HTML / or something else after your PHP code
• no need to use "" to wrap strings in case that you don't have any variables inside a string, you can use '' instead, PHP interpreter does not need to check in that case whether there are any variables in the string or not
• you can replace echo $e->getMessage; die() by simplier exit($e->getMessage());
• I added salt generation to your code $salt = md5(uniqid(null, true));
• I added password hashing to your code by $password = hash('sha256', $password . $salt);

The whole code hereunder:

<?php
try {
 $handler = new PDO('mysql:host=localhost;dbname=s','root', '*');
 $handler->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch (PDOException $e){
 exit($e->getMessage());
}
$name = $_POST['name'];
$username = $_POST['username'];
$email = $_POST['email'];
$password = $_POST['password'];
$salt = md5(uniqid(null, true));
$password = hash('sha256', $password . $salt);
$sql = '
 INSERT INTO userinfo 
 (name ,username, email, password, salt) 
 VALUES 
 (:name,:username,:email,:password, :salt)
';
$query = $handler->prepare($sql);
$query->execute(array(
 ':name' => $name,
 ':username' => $username,
 ':email' => $email,
 ':password' => $password,
 ':salt' => $salt
));

• 200_success: Excellent observation, that salting and hashing are essential, you have made.
• user2981256: Excellent answer. But isn't password_hash safer?
• user40964: @user2981256 You can use password_hash as well, safety depends on used hashing algorithm. You can choose these values in both methods so the safety is about the same.
• bdsl: Don't use sha256 for passwords. It's much too fast and therefore easy to attack. Use bcrypt, scrypt or PBKDF2.

Answer 2

Yoda has some good points, and I agree with it being safe. I noticed you mentioned validating, but I figured I'd add my 2 cents for future readers that may come across this question. A few things jump out at me from the following:

$name = $_POST['name'];
$username = $_POST['username'];
$email = $_POST['email'];
$password = $_POST['password'];

You should check that all fields have been posted, and stop processing if there is information missing. An example of this would be:

$name = isset($_POST['name']) ? $_POST['name'] : null;
if($name==null) { 
 exit(); 
}

This information is being entered into the database without any checks, which can lead to bad data being stored. I'm assuming that there may be checks client side (browser), but those checks are by no means fool-proof. Javascript could be disabled, rending those checks useless. There is required for HTML5, and some browsers have implemented that functionality. However, not all have, and many users will inevitably be using the older versions that haven't. What if the name and username are blank, or the name is 31&^5. Checking the length and content should be done both on the client and the server.

+1 for prepared statements!