0

I have finally finished my login and register form WITH e-mail verification and I am SO happy! I am just not sure if my form is secure enough. I've used htmlspecialcharacters() on all user inputs and used placeholders in SQL queries. Is this enough to stop attacks on my form? Please help, as all answers are appreciated. Thanks! This is the code for my register form: http://pastebin.com/FWsDqeKs

<?php
include_once 'compat.php';
$user = 0;
$pass = 0;
$mail = 0;
if($_SERVER['REQUEST_METHOD'] == 'POST' && !empty($_POST['mail']) && !empty($_POST['user']) && !empty($_POST['pass'])) {

    function an($subject) {
        if(ctype_alnum($subject)) {
            return true;
        }
    }

    if(an($_POST['user']) && strlen($_POST['user']) <= 20) {
        $user = htmlspecialchars($_POST['user']);
    }
    else {
        echo 'username is invalid (a-z A-Z 0-9 and less than 20 characters, please)<br>';
    }
    if(filter_var($_POST['mail'], FILTER_VALIDATE_EMAIL) && strlen($_POST['mail']) <= 255) {
        $mail = htmlspecialchars($_POST['mail')];
    }
    else {
        echo 'email is invalid<br>';
    }
    if(strlen($_POST['pass']) <= 255) {
        $pass = htmlspecialchars($_POST['pass']);
    }
    else {
        echo 'password must be less than 255 characters<br>';
    }
    if($pass && $user && $mail) {
        $hash = md5(rand(0, 1000));
        require_once('db.php');
        $sth = $db->prepare("
                                INSERT INTO user_info (username, password, email, hash)
                                VALUES (:username, :password, :mail, :hash)
                ");
        $sth->bindValue(":username", $user, PDO::PARAM_STR);
        $sth->bindValue(":password", password_hash($pass, PASSWORD_DEFAULT), PDO::PARAM_STR);
        $sth->bindValue(":mail", $mail, PDO::PARAM_STR);
        $sth->bindValue(":hash", $hash, PDO::PARAM_STR);
        $success = $sth->execute();
        echo 'You have been successfully signed up :)! To activate your account, click the link in your e-mail account (check your spam box if you don\'t get the e-mail!)<br>
                The last step to activating your account is clicking this link!:<br>
                http://localhost/verify.php?mail=' . $mail . '&hash=' . $hash . '<br>
                ';
    }
}
?>
Chris Forrence
  • 10,042
  • 11
  • 48
  • 64
crgsqdmn
  • 639
  • 1
  • 5
  • 7
  • if you would provide us some example code, so we can take a look at it. would be very useful – rsz May 12 '14 at 23:22
  • What do you mean by "placeholders"? Do you mean parameterized prepared statements? – siride May 12 '14 at 23:23
  • How you perform SQL-queries is more important in this case. I hope you don't use `mysql_*` functions? – display-name-is-missing May 12 '14 at 23:23
  • 3
    Using `htmlspecialchars` to insert data into a database is a sure sign you don't really know what you're doing... Prepared statements are all you need. `htmlspecialchars` is for outputting to the browser. – Niet the Dark Absol May 12 '14 at 23:24
  • @NiettheDarkAbsol: You are making assumptions. It might turn out that the OP actually does that, but it doesn't follow from the question. – Jon May 12 '14 at 23:26
  • Hi, rsz! I've updated the post, providing a pastebin link to the register form on my website. Thank you! :) – crgsqdmn May 12 '14 at 23:26
  • @NiettheDarkAbsol: functions like htmlspecialchars could potentially be useful in preventing 2nd order injection attacks. There are probably better ways, though. – siride May 12 '14 at 23:28
  • Hey @user3344396 - I edited your question to include your PHP code into the question. Something I noticed: you've got a typo on line 18; you mixed up the ending bracket and parenthesis. – Chris Forrence May 12 '14 at 23:30
  • Hi @ChrisForrence! Sorry that I haven't included a code block, I haven't used this site much since I signed up. Thanks for noticing the typo and fixing my post! – crgsqdmn May 12 '14 at 23:31
  • Not so fast ;-) You're not quite done yet. Don't use MD5 for password storage, it's broken, *busted* you know... **unsafe to use**. Use [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). – Funk Forty Niner May 12 '14 at 23:35
  • 1
    Hio @Fred -ii! That MD5 is not for storing the password, but in fact, storing a hash for the e-mail verification link. As you can see in the line including "password_hash($pass, PASSWORD_DEFAULT)", I'm using password_hash to store passwords. – crgsqdmn May 12 '14 at 23:37
  • @user3344396 Silly me, my mistake. Good stuff then ;-) Well, you should be good to go then! The other comments are also useful. – Funk Forty Niner May 12 '14 at 23:38
  • @siride, mucking about with the data before you store it might be a good idea as long as there is no possibility that it will be displayed but on a web page. – Dan Bracuk May 12 '14 at 23:47
  • 1
    This question appears to be off-topic because it would be more appropriate on http://codereview.stackexchange.com/ – SilverlightFox May 13 '14 at 09:08

0 Answers0