[kdye101]

Hi everyone, first time poster here. I am new to PHP and self-taught with a couple of books. This is my first ever PHP project and I am working on a registration form. I have not started in on the MySQL portion yet because I want to make sure the error-checking is right before I move on. These are the fields and the rules I would like for each.

Username (6-12 chars, alphanumeric only. Cannot start with a number)
Password (6-12 chars, alphanumeric requiring 1 letter (any case) and 1 number)
Password Confirmation
Email (I used the built in PHP function to check this. Is it bad?)
Email Confirmation

Here is the only function that is called from outside the file. I copied it from a book. I am aware that there will be a different function for when I start in with the MySQL inserts, but I'm not ready for that yet.

function sanstring($input) {
    $input = stripslashes($input);
    $input = htmlentities($input);
    $input = strip_tags($input);
    return $input;
}

And here's the page content.

<?php

require_once 'header.php';
require_once 'config.php'; #for salts.
require_once 'code.php'; #for sanstring()

if (isset($_POST['submit'])) {
    /*
     * ----------ERROR CHECKING HERE
     */
    $error = $username = $password = $confpassword = $email = $confemail = "";
    if ($_POST['username']) {
        $username = sanstring($_POST['username']);
        if (strlen($username) > 12)
            $error .= '<div>The username you entered is too long.  Username must be 6 to 12 characters.</div>';
        if (strlen($username) < 6)
            $error .= '<div>The username you entered is too short.  Username must be 6 to 12 characters.</div>';
        if (preg_match('/[^a-z0-9_]/', $username))
            $error .= '<div>The username you entered contains invalid characters.  Username may contain A-Z, a-z, 0-9, and _.</div>';
    } else {
        $error .= '<div>You did not enter a username.  Please try again.</div>';
    }if ($_POST['password']) {
        $password = sanstring($_POST['password']);
        if (strlen($password) > 12)
            $error .= '<div>The password you entered is too long.  Password must be 6 to 12 characters.</div>';
        if (strlen($password) < 6)
            $error .= '<div>The password you entered is too short.  Password must be 6 to 12 characters.</div>';
        if (preg_match('/[^a-z0-9]i/', $password))
            $error .= '<div>The password you entered contains invalid characters.  Password may contain A-Z, a-z, 0-9, and !@#$%^&*()</div>';
    } else {
        $error .= '<div>You did not enter a password.  Please try again.</div>';
    }if ($_POST['confpassword']) {
        $confpassword = sanstring($_POST['confpassword']);
        if (strcmp($password, $confpassword))
            $error .= '<div>The password confirmation does not match your password.  Please try again.</div>';
    } else {
        $error .= '<div>You did not enter your password confirmation.  Please try again.</div>';
    }
    if ($_POST['email']) {
        $email = sanstring($_POST['email']);
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) $error .= '<div>The email you entered is not valid.</div>';
    } else {
        $error .= '<div>You did not enter your email address.  Please try again.</div>';
    }if ($_POST['confemail']) {
        $confemail = sanstring($_POST['confemail']);
        if (strcasecmp($email, $confemail))
            $error .= '<div>The email confirmation does not match your email.  Please try again.</div>';
    } else {
        $error .= '<div>You did not enter your email confirmation.  Please try again.</div>';
    }
    if ($error) {
        echo $error;
    } else {
        /*
         * ---------CODE IF NO ERRORS EXIST 
         */
        $password = md5($salt1 . $password . $salt2);  #salts are in config.php
    }
} else {
    echo <<< _END
<div>
    <form action="register.php" method="post">
    Desired Username: <input type="text" maxlength="16" name="username" /><br />
    Desired Password: <input type="password" maxlength="16" name="password" /><br />
    Confirm Password: <input type="password" maxlength="16" name="confpassword" /><br />
    Email Address: <input type="text" name="email" /><br />
    Confirm Email: <input type="text" name="confemail" /><br />
    <input name="submit" type="submit" value="Submit" />
    </form>
    </div>
_END;
}

/*
 * TODO:
 * Function to ensure username starts with a letter.
 * Function to ensure password contains one letter and one number.
 * 
 */
?>

Thank you for your input!

[CTphpnwb]

The sanstring function is unnecessary if you use prepared statements. Since you're just beginning it's much better to learn them than to try to learn to fully sanitize user input.

http://www.dreaminco...duction-to-pdo/

[kdye101]

Thanks for the info. I have read about using the PREPARE statement, and the stuff that I have read says you should still sanitize any user input, because you never know what someone will try to do to your code. You don't strip anything from the raw user input?

What about my PHP, how can it be better? What concepts am I not grasping that will make my life easier as things get more complicated? Is this the standard way to do this type of error checking?

[Dormilich]

kdye101, on 18 January 2012 - 05:01 AM, said:

I have read about using the PREPARE statement, and the stuff that I have read says you should still sanitize any user input, because you never know what someone will try to do to your code. You don't strip anything from the raw user input?

when using Prepared Statements there is no need of sanitising for SQL Injections. Prepared Statements are immune thanks to the content-code separation. what you still have to do is prevention of XSS attacks, which are usually done before outputting the content to the browser.

kdye101, on 18 January 2012 - 05:01 AM, said:

What about my PHP, how can it be better? What concepts am I not grasping that will make my life easier as things get more complicated? Is this the standard way to do this type of error checking?

there are some issues, though.
hashing: MD5 is considered an insecure hashing algorithm (it is relatively easy to systematically find a string with the same hash value). better algorithms are the SHA2 family (sha256), ripemd (ripemd160) or whirlpool. for employing salts, I recommend the HMAC variant available through hash_hmac().

error messages: error messages are a means for hackers to gather attack vectors. usually it suffices to say that either username or password are incorrect. if you want you can use Javascript to aid the user in creating valid names. personally, I would not agglomerate error messages, just break out on the first occurence.

(email) check: while filter_var() is nice filter_input() is better. generally you would have an issue if the $_POST fields do not exist (unless you explicitly test for their existance before), which generates a warning/notice. filter_input() does not need such a test and will just return NULL if the field was not set. email address* input should also not be sanitised, otherwise the user may input an invalid address that becomes valid by sanitisation. this may not be an issue for logging in, but if you want to send an email to the address in your DB, you may end up using the wrong address. similarly, you can check pretty much all of your constraints in a single RegExp.
additionally, when the name length is supposed to be 12 at max, why using maxlength="16"?

stripslashes() is unnecessary if input escaping is disabled in the php.ini (magic_quotes_gpc, I think)


* - while email addresses are case-sensitive by definition, ISPs usually handle them case insensitive to prevent confusion and mislead messages.

This post has been edited by Dormilich: 17 January 2012 - 11:54 PM

[JackOfAllTrades]

Quote

error messages: error messages are a means for hackers to gather attack vectors. usually it suffices to say that either username or password are incorrect. if you want you can use Javascript to aid the user in creating valid names. personally, I would not agglomerate error messages, just break out on the first occurence.

I would go so far as to say you should not say WHICH is incorrect; simply "Username and/or Password is incorrect". Giving up that the username is valid is more information that the attacker can use in his attempts.

[Dormilich]

I should have used quotes to make that clear, because that's what I had in mind.

thanks for pointing it out

[kdye101]

Wow thanks for the information. The security article that I saw mentioned error messages during login attempts, but I should prevent them from generating during website registration? I also found a great website via another forum, regular-expressions.info, and will be incorporating better checking this way. Regular expressions are confusing!

Another change I will be making is to store the messages in an array and using an unordered list to display them.

So the MD5 hashes with two random salts on both sides are not secure? I know of the dictionary MD5 databases, but that's what I thought salts were for. Are you saying that they are decrypting the encryption, not simply looking up the hash in a database?

[Dormilich]

they are not decrypting, they use collisions (two different inputs that have the same hash value). (see also the wiki article about md5). salts make it more difficult to get a collision, but as long as the algorithm is prone to collisions, it will work out sooner or later.