3
\$\begingroup\$

Does my PHP look secure enough for a sign up form?

<?php
$con=mysqli_connect("host","user","password","db_name");
$sql="INSERT INTO users values (?, ?, ?)";
if ($stmt = mysqli_prepare($con,$sql)) {
  mysqli_stmt_bind_param($stmt, "sss", 
    $_POST["username"], $_POST["pwd"], $_POST["email"]);
  if (mysqli_stmt_execute($stmt)) {
    echo "User added!";
  }
}
\$\endgroup\$
8
  • 3
    \$\begingroup\$ From PHP side of view: you might want to check if connection is successful first. It's important to check if all variables are valid (not empty, matching something). Also someone might spoof form and pass an array instead a string. From security point of view: missing salting and hashing of the password. You might also want to make sure that mail and username are unique (but you can do that using sql unique keys). \$\endgroup\$
    – Misiur
    Dec 14, 2013 at 23:37
  • \$\begingroup\$ It depends on your php version and available plugins. PHP 5.5 - php.net/manual/en/function.password-hash.php . Earlier versions php.net/manual/en/function.crypt.php - I recommend blowfish algorithm. Take a look on second comment, it shows a quite secure way to generate salt. \$\endgroup\$
    – Misiur
    Dec 14, 2013 at 23:45
  • \$\begingroup\$ @Misiur 2nd to top or 2nd to bottom? \$\endgroup\$ Dec 14, 2013 at 23:57
  • 3
    \$\begingroup\$ @Misiur: Why don't you turn your comments into an answer? There is enough there to be a code review. \$\endgroup\$
    – ChrisWue
    Dec 15, 2013 at 8:03
  • 1
    \$\begingroup\$ @SaggingRufus Yeah, I'm salting and hashing. \$\endgroup\$ Jan 10, 2014 at 21:37

1 Answer 1

3
\$\begingroup\$

It looks like you have a good and solid Insert, probably pretty safe, but this still allows a user to enter data into the database that could be run at a later time, you should still check the input coming in before inserting it into the Database even though it has been "prepared".

The prepare just means that the insert will not break anything, but if you run a SQL Query on the information in that column where someone has entered SQL injection Characters it could still crash the database.

Please make sure the input is safe all the way around. Alert the user that they cannot enter certain characters but be aware you are asking for an e-mail address there too. The Password should be encrypted, not sure whether you should do that here or in the Database itself, probably in the PHP, but that is another review altogether.

\$\endgroup\$

Your Answer

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

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