I'm just wondering if this code is good against SQL injection attacks. I don't know how to check if is good. Also I would like to know if is good how I'm working or this is just bad practice?


if (isset($_POST['register'])) {

$username = $_POST["username"];
$email = $_POST["email"];
$password = $_POST["password"];
$salt = "Not important";
$password_hash = crypt($password, "$2y$". $salt);

if (strlen($username) > 20) {
    echo "Username is too big";
}elseif (strlen($password) > 32) {
    echo "Password is too big";
}elseif (strlen($email) > 100) {
    echo "E-Mail is too big";
}elseif(strlen($username) == 0 || strlen($password) == 0 || strlen($email) == 0){
    echo "Fill every field";
    $sth_username = $dbh->prepare("SELECT username FROM user WHERE username = :username");
    $sth_username->bindParam(":username", $username);
    $sth_email = $dbh->prepare("SELECT email FROM user WHERE email = :email");
    $sth_email->bindParam(":email", $email);
    if ($result = $sth_username->fetch(PDO::FETCH_OBJ)) {
        print $result->username . "The username is already is use";
    }elseif ($result = $sth_email->fetch(PDO::FETCH_OBJ)){
        print $result->email . "That e-mail is already in use";
        $sth = $dbh->prepare("INSERT INTO user (username, password, email, salt) VALUES (:username, :password, :email, :salt)");
        $sth->bindParam(":username", $username);
        $sth->bindParam(":password", $password_hash);
        $sth->bindParam(":email", $email);
        $sth->bindParam(":salt", $salt);
        $sth = $dbh->prepare("INSERT INTO stats (strength,agility,intelligence) VALUES (10,10,10)");

        echo "You have been registered";
  • 1
    \$\begingroup\$ Looks good to me \$\endgroup\$
    – Eric Petroelje
    Jul 10, 2013 at 21:02
  • \$\begingroup\$ Since you're using a Blowfish hash, make sure the format is correct. \$\endgroup\$
    – Blender
    Jul 10, 2013 at 21:05

1 Answer 1


Looks O.K. to me as well, as far as the SQL injection goes.

However, I'd suggest checking the return value of $sth->execute();, so you capture possible errors (i.e., DB being down).

You might also consider doing a case insensitive search for the existing usernames and e-mails, since "[email protected]" is the same address as "[email protected]" and users "Person" and "person" are hard to distinguish.

I think empty($string) is more advisable than strlen($string) == 0.

Last, but not least, adding a "retype your password" field would be a good security against user mistyping what he wants for a password and then ending with a useless account (or having to use "I forgot my password" before the very first login).

  • \$\begingroup\$ The people at UX.SE disagree with you on offering "retype your password" (ux.stackexchange.com/q/20953/16833) \$\endgroup\$
    – Brian
    Jul 12, 2013 at 19:54
  • \$\begingroup\$ @Brian I don't agree with you. Yes, the "retyping the password is bad, without a real explanation" answer is the accepted one, and it got the most votes. However, the other answers and the comments throughout that thread are far from consensus, so I don't see this to be "the people at UX.SE" disagreeing (nor agreeing) with me, or among themselves. \$\endgroup\$ Jul 12, 2013 at 22:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.