Create user script

[ed89]

Hi there, wonder if anyone can look over the below and check for any potential errors, vulnerabilities or just a better, neater way of doing it. I am getting to grips with PHP so appreciate any comments on how I can improve what I've done thus far.

The script takes the inputted username and password, checks that they do not match, or that either is less than 3 characters or more than 10. Also strips any html from the username field to prevent XSS (though max char is 10!)

So, simple HTML form:

<form action="submit.php" method="post" >
Username (min 3 char, max 10 char):<br>
<input name="username" type="text"><br> 

Password (min 3 char, max 10 char)<br>
<input name="password" type="password"><br>
<input name="submit" type="submit" value="submit">

</form>

And the submit.php file which the form will use to process the input:

<?php

$username = $_POST['username']; //get submitted username from form
$password = $_POST['password']; //get submitted password from form

if (($password == NULL) || ($username == NULL)) {
        echo "Missing username or password."; //stop here if a u/pw was not entered
        die();
} 

if ($username == $password) {
        echo "Username and password must not match!";
        die();
}

if (strlen($username) < 3 || strlen($username) > 10) { // if USERNAME is less than 3 or more than 10...
    echo "Your username is less than 3 or more than 10!";
    die(); //stop here if username is less than 3 or more than 10
} elseif (strlen($password) < 3 || strlen($password) > 10) { // if PASSWORD is less than 3 or more than 10...
    echo "Your password is less than 3 or more than 10!";
} else {
    echo "Thank you for signing up. Your username is <b>" . strip_tags($username) . " </b>and your password is <b>$password</b>";    
}

echo "<br /> <br />";

$password = md5($password); //encrypt the password using md5

echo "Using md5 encryption, your password would be entered into the database as the following $password";

//do something, like put info into db

?>

I think die() perhaps isn't what I shouldn't be using the stop the script after a condition?

[darkecho]

You might want to show the form again if you encounter an error instead of just killing the page.

[gorda001]

You might want to do this:

 $username = trim($_POST['username']);
$password = trim($_POST['password']); 
  

That removes any spaces,tabs, etc from the start and end of the username and password. Useful for if the user pressed space by mistake.

[unclekyky]

That removes any spaces,tabs, etc from the start and end of the username and password. Useful for if the user pressed space by mistake.

And to prevent someone from registering 5 spaces as his username. ;)

Instead of "$password == NULL" I would use "empty($password)" to check and see if the password/username was entered.

You should remember that md5 is neither an encryption nor secure. It is a hash, sometimes called one-way "encryption." You should never use md5 to store a password and still expect security. Make sure to at least salt the hash. I've seen people do something like this (which ads a bit of security in regards to brute force/lookup tables, but does nothing for collisions):

 <?php
$password = "abc123";
// "salt-string" is  the salt.  Make sure to remember what the salt is (don't use time() without storing it, for example).  You need it for login validation.
$password_db = md5("s41t-S7r1n9".$password);
?>  

[hot_cakes]

You might want to do this:

 $username = trim($_POST['username']);
$password = trim($_POST['password']); 
  

That removes any spaces,tabs, etc from the start and end of the username and password. Useful for if the user pressed space by mistake.

I'd actually suggest using two fields: "password" and "re-enter password". This way the user can add spaces at either end until the cows come home and it won't be by accident. I *hate* websites that place arbitrary restrictions on passwords.

EDIT: this also makes sure that the user doesn't create an account that they're locked out of from day 1 due to a typo when choosing their password!

Edd

[ed89]

Wow, 4 responses already!

@darkecho - good idea. I won't be using the form anywhere, it's just to refresh myself with PHP. It's my first scripting language so I am trying to make myself think like a programmer for the moment.

@gorda- Thanks for that. Useful function I have used before.

@unclekyky - As above. I will look into "salting" the md5 string for added security.

@hot_cakes - As mentioned, I won't be using the form anywhere so it's just for practice. :D

[MaradoX-]

And for checking your input on forbidde characters, like your userename you can use this function ;):

 function PhraseString($string) { 
$string = preg_replace("/[^a-zA-Z0-9_ -]/", "", $string); 
return $string; 
} 
  

It'll remove all chars except a-b A-Z 0-9 and - and _ ;)

*function made by a friend of me Shane, so all credits go to him :)

[Reaper]

MaradoX you really should use preg_match and make them change it if any results turn up. Having it so that it simply replaces every character could create unwanted usernames and then people getting upset. Simply use preg_match and state that only common characters without accents are allowed.