Secure Registration

[White_Lily]

Hey,

 

I have written a secure registration form, and I have wondered could could be done to make even more secure. (I want to try and avoid capchas if possible.)

 

Here is my current code:

 

<?php

$register = $_POST["register"];
$name = mysql_real_escape_string($_POST["createName"]);
$username = mysql_real_escape_string($_POST["createUsername"]);
$password = $_POST["createPass"];
$rePass = $_POST["createRePass"];

if(!empty($register)){

foreach($_POST as $key => $value){
if(empty($value)){
$error = "<li class='error'>You cannot submit the form while it is empty.</li>";
}
}
if(!empty($name) && !empty($username) && !empty($password) && !empty($rePass)){
if($password != $rePass){
$error .= "<li class='error'>The passwords you typed do not match.</li>";
}

if(strlen($username) < 6 || strlen($username) > 30){
$error .= "<li class='error'>Your username is either too short or too long. Usernames can only be a minimum of 6 characters and no longer than 30 characters.</li>";
}

if(strlen($password) < 6){
$error .= "<li class='error'>Your password must be longer than 6 characters.</li>";
}

$compare = select("users") or die(mysql_error());
$use = mysql_fetch_assoc($compare);

if($username == $use["user_username"]){
$error .= "<li class='error'>That username is already in use. Choose another.</li>";
}

if(empty($error)){

$string = sha1(uniqid(rand(), true));
$salt = substr($string, 0, 3);
$hash = hash('sha256', $password);
$encrypt = $salt.$hash;

$newUser = insert("users", array("", $name, $username, $encrypt, $salt, $hash));

if($newUser){
session_start();
$_SESSION["user"] = $username;
$success .= "<li class='pass'>User added. Username: ".$_SESSION["user"]."</li>";
}else{
$error = "<li class='error'>".mysql_error()."</li>";
}
}
}

}

?>

<div class="heading" style="margin:0 0 5px 0;">Register</div>
<ul>
<?php

if(!empty($error)){
echo $error;
}if(!empty($warn)){
echo $warn;
}if(!empty($success)){
echo $success;
}

?>
</ul>
<form action="" method="POST">
<label>Name</label>
<div class="clear"></div>
<input type="text" name="createName" value="<?=$name?>" class="fields" />
<div class="clear"></div>
<label>Username</label>
<div class="clear"></div>
<input type="text" name="createUsername" value="<?=$username?>" class="fields" />
<div class="clear"></div>
<label>Password</label>
<div class="clear"></div>
<input type="password" name="createPass" class="fields" />
<div class="clear"></div>
<label>Repeat Password</label>
<div class="clear"></div>
<input type="password" name="createRePass" class="fields" />
<div class="clear"></div>
<input type="submit" name="register" value="Sign Up" class="buttons" />
</form>

[Muddy_Funster]

• you seem to have no validation at all, for anything, other than char count on username and password.
  
• you double check if the fields are empty, although on the second check the $name and $username will likely have something in it generated from the fact that you have run mysql_real_escape_string on them far too early.
  
• your sql function is poorly written for the insert method as you seem to be foreced to insert nothing into the auto inc id field of the user table
  
• there is no point in storing the hash, the salt and the combines hash and salt. either store the combination, or store each on their own, doing it your way leads to redundant data.
  
• I'm pretty sure, from what I can see, that your select() function will likely be doing a SELECT * from the users table, which is needlesly returning additional information (about your users) from the database - such as the users password info. This is inefficient and potentialy insecure.
  

[White_Lily]

1) What other validation should be done? just saying that there isn't any doesn't really help me much... i.e: should i be using expressions on certain inputs?

 

2) Not entirely sure if you noticed, but every variable needs a value for it to continue with the checks and user creation, hence the "&&" after every !empty().

 

3) What do you suggest I do about the insert query then? (I'm not a mind-reader unfortunately)

 

4) No data is redundant in my table. It will all get used. (Believe it or not but before I build my sites i plan what the site will use and how any data will be used)

 

I have changed the select query to only select the username field.

[Muddy_Funster]

ok, so were going with a flippent theme then, I can do flippent.

1. Checking that some meaningful text, other than just 6 spaces for example, has been entered for the username would probably be a good place to start.
   
2. Not entierly sure if you noticed, but you already check all the POST varibles if they are empty before then checking if the variables you poloulated from the POST array are also empty. Or was that part of the plan?
   
3. Make it work properly would be my suggestion. Have it take two array sets - one for columns the other for values
   
4. Storing the same information multiple times in the same system is called creating redundant data. Perhaps part of your planning should be normalisation?
   

Now if you want to keep up this tone in the thread you can find someone else to play with. I'm not puting up with you being arsit when this is entierly for your benefit.

[White_Lily]

The part where i check if all fields have data is that the script does not come up with a whole bunch of errors (hopefully making it easier on the user :/ )

 

Use of data is whats planned based on what the client wants and how (s)he wants to use it, it is also based on how much they have in their budget which allows me to plan how much work i can do with what they give me.

[Muddy_Funster]

Checking the $_POST array directly is still doing the same job as checking the variables after it. You're double working it. It just doesn't make sense to do that. Especialy as you are applying mysql_real_escape_string() to two of the checks before checking if they are empty.

Clients should never have a say in the structure of the system, if they were qualified to make that decission they wouldn't be paying someone else to come in and do it. That the data needs used is fair enough, but it only needs to be stored in a single location in a single form for that to happen, there is no need to have the same data stored in full in one location and then in multiple bits in another. you should either store the bits and build the full thing when requesting the data from the system or you should store the whole thing and then break it into the bits you need when requesting it from the system.

[White_Lily]

You keep saying the mysql_real_escape_string() is done too early, but it still checks them if they are empty and displays an error if they are empty... so im not extirely sure what your saying. However, feel free to see for yourself with the forum developnent link in my signature thing.

[Muddy_Funster]

I'm just saying that mysql_real_escape_string() should only be run last thing before sending the string to the database. I'd personaly probably be using a

filter_vars($name, FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW);

rather than the mysql_real_eascape_string at the point where you are. The issue will only be that if you ever need to update the script, either in two weeks or in six months, and you want to start using those variables for other things, like showing a nice little personalised message or something, then it's not always going to look all that nice if the user name is something like Death's Too Slow.

[cyberRobot]

Other than needing to be six characters, are there any rules for the password? For example, does it need to include at least one special character and/or number? If so, checks can be added to make sure the password meets whatever criteria is in place.

 

 

When comparing the username against other usernames in the database, how does it handle case? Unless the username field in the database is set to be case sensitive, "MyUsername" and "myusername" will be considered the same. PHP, on the other hand, will treat them as different usernames. To prevent a potential conflict, the PHP test could be modified.

 

<?php
if(strtolower($username) == strtolower($use["user_username"])){
?>

 

 

As a side note, the <label> tags are incomplete. An "id" attribute needs to be added to the <input> tags and a matching value needs to be added to the "for" attribute in the <label> tags. For more information, see:

http://www.cyberscor...-the-label-tag/

[cyberRobot]

Have you considered the Honeypot technique? This can help prevent form spam without getting in the way of most users. More information can be found here:

http://haacked.com/a...ot-captcha.aspx

Edited December 19, 2012 by cyberRobot

[mrMarcus]

I made some notes in another thread regarding an alternative to CAPTCHA: http://forums.phpfreaks.com/topic/235697-contact-form-with-math-captcha/?do=findComment&comment=1397557

 

If OP is interested.

Edited December 19, 2012 by mrMarcus

[White_Lily]

@cyberRobot; I have added an expression which i commonly use for passwords which basically says that you have to have 1 uppercase letter, 1 digit, and 1 lowercase letter and a minimum of 6 characters. and i have added the stuff about the labels and inputs

 

@cyber & marcus; I will have a look at thos captcha examples and will post later what I have done.

Edited December 19, 2012 by White_Lily

[mrMarcus]

@cyber & marcus; I will have a look at thos captcha examples and will post later what I have done.

 

To clarify, these are alternatives to CAPTCHA.

[White_Lily]

yeah lol sorry. Marcus, a question about your alternative (lol) to captchas, shouldnt $_POST["start_time"]; be $_SESSION["start_time"];?? or am i to actually have another field in my form a with the time in it?

[White_Lily]

Your posts on your provided thread also state to adjust the time it takes to fill the form out, problem with this though is that people will have to take some time to think of a username... so im not sure if the time method would work...

[Jessica]

Your posts on your provided thread also state to adjust the time it takes to fill the form out, problem with this though is that people will have to take some time to think of a username... so im not sure if the time method would work...

 

You want it to take a while, not be quick. If it only takes them 1 second, that's a bot.

[Muddy_Funster]

The idea should be that it would be a minimum time, as bots eat through forms way quicker than people do, so thinking about a user name would actualy be better for the person filling out the form because a bot never stops to think, it just batters options in untill something givs

 

Edt : yeah - what Jessica said :/ damn my slow fingers :'(

Edited December 19, 2012 by Muddy_Funster