Checking Password Length with IF Statement Not Working! (Register Script)

[chaseman]

This is my registering script:

<?php
include('connectvars.php');

$user_email = strip_tags(trim($_POST['email']));
$firstname = strip_tags(trim($_POST['firstname']));
$lastname = strip_tags(trim($_POST['lastname']));
$nickname = strip_tags(trim($_POST['nickname']));
$password = strip_tags($_POST['password']);
$repassword = strip_tags($_POST['repassword']);
$dob = $_POST['dob'];
$find_us_question = strip_tags(trim($_POST['find_us_question']));


if (isset($_POST['submit_signup'])) {

if ((empty($user_email)) || (empty($firstname)) || (empty($lastname)) || (empty($nickname)) || (empty($password)) || (empty($dob))) {

	echo "Please fill out all the fields!";

	} else {
						// check char length of input data
						if (($nickname > 30) || ($firstname > 30)  || ($lastname > 30) || ($user_email > 50)) {

							echo "Your nickname, first- and/or lastname seem to be too long, please make sure you have them below the maximum allowed length of 30 characters!";

						} else {

						// check password char length
						if (($password > 25) || ($password < 6)) {
							echo "Your password must be between 6 and 25 characters!";

							} else {
								// encrypt password
								$password = sha1($password);
								$repassword = sha1($repassword);


							 if ($password != $repassword) {
										echo "Please make sure your passwords are matching!";

								} else {

									$dbc = mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME);

									$query = sprintf("INSERT INTO user (firstname, lastname, nickname, password, email, dob, doj) VALUES ('%s', '%s', '%s', '%s', '%s', '%s', now())",
									mysqli_real_escape_string($dbc, $firstname),
									mysqli_real_escape_string($dbc, $lastname),
									mysqli_real_escape_string($dbc, $nickname),
									mysqli_real_escape_string($dbc, $password),
									mysqli_real_escape_string($dbc, $user_email),
									$dob);



									mysqli_query($dbc, $query);

									mysqli_close($dbc);

									echo "You have been successfully registered!";

							}

					}
				}
			}
	}	

?>

 

A bunch of nested if statements, the read-ability gets worse after a while, I'm new to programming so I don't know if there's a better more read-able solution.

 

Anyway, every time I try to sign up it's printing out the echo message:

"Your password must be between 6 and 25 characters!"

 

Which derives from:

 

// check password char length
						if (($password > 25) || ($password < 6)) {
							echo "Your password must be between 6 and 25 characters!";

							} else {

 

EVEN if I stay between 6 and 25 characters it's still printing out this error message, let's say I have a password of 8 characters, and I've entered everything else correctly, it's still giving me all the time this error message, and I can not figure out why.

[BlueSkyIS]

to get the length of a string use strlen($var)

[Psycho]

Because you are not checking the LENGTH of the password. Your condition is trying to compare a string to a number. I think a string will resolve to 0 in that instance.

 

Here is some revised code. There is a lot more I would do with this though. A couple of changes I made are:

 

1. Moved the code to process the input to be after the condition check to see if data was posted. No need to try and process the data if you haven't even verified there was posted data.

2. Compared the password and repeat password before hashing it. You only need to hash the password once. No need to hash before comparing.

3. Implemented elseif statements to remove all the nested loops.

 

Other things you could/should do.

 

1. Do NOT use striptags on input unless magic quotes is turned on. Look at the manual for strip_tags() for code you can implement to automatically strip tags only when needed.

2. You could set a flag at the start - then determine ALL the errors and display them to the user. As you have it now after the first error is encountered it only gives the user that particular message even if other errors exist.

3. Add more error handling for the data - such as checking the date field to ensure it is really a date value.

 

include('connectvars.php');

if (isset($_POST['submit_signup']))
{
    $user_email = strip_tags(trim($_POST['email']));
    $firstname  = strip_tags(trim($_POST['firstname']));
    $lastname   = strip_tags(trim($_POST['lastname']));
    $nickname   = strip_tags(trim($_POST['nickname']));
    $password   = strip_tags($_POST['password']);
    $repassword = strip_tags($_POST['repassword']);
    $dob        = $_POST['dob'];
    $find_us_question = strip_tags(trim($_POST['find_us_question']));
    
    //Check that all required fields were posted
    if ((empty($user_email)) || (empty($firstname)) || (empty($lastname)) || (empty($nickname)) || (empty($password)) || (empty($dob)))
    {
        echo "Please fill out all the fields!";
    }
    //Check length of data
    elseif (($nickname > 30) || ($firstname > 30)  || ($lastname > 30) || ($user_email > 50))
    {
        echo "Your nickname, first- and/or lastname seem to be too long, please make sure you have them below the maximum allowed length of 30 characters!";
    }
    // check password char length
    elseif (strlen($password)>25 || strlen($password)<6)
    {
        echo "Your password must be between 6 and 25 characters!";
    }
    //Check that passwords match
    elseif($password != $repassword)
    {
        echo "Please make sure your passwords are matching!";
    }
    else
    {
        // encrypt password
        $password = sha1($password);
        $dbc = mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME);
        $query = sprintf("INSERT INTO user (firstname, lastname, nickname, password, email, dob, doj)
                          VALUES ('%s', '%s', '%s', '%s', '%s', '%s', now())",
                            mysqli_real_escape_string($dbc, $firstname),
                            mysqli_real_escape_string($dbc, $lastname),
                            mysqli_real_escape_string($dbc, $nickname),
                            mysqli_real_escape_string($dbc, $password),
                            mysqli_real_escape_string($dbc, $user_email),
                            $dob);
        mysqli_query($dbc, $query);
        mysqli_close($dbc);
        echo "You have been successfully registered!";
                }
            }
        }
    }
}

[chaseman]

Thank you guys, and special thanks to you mjdamato for rewriting the code, it looks much more elegant now.

 

What would you recommend to use instead strip_tags, so it's secure?

[Psycho]

What would you recommend to use instead strip_tags, so it's secure?

 

strip_tags() has nothing to do with security. You are already "securing" the data from sql injection by using mysqli_real_escape_string() which is exactly what you should be doing. The reason you would use strip_tags() on user input is that some servers have "magic quotes" turned on.

 

The purpose of magic quotes was to automatically escape the data. But, it has several problems. For one, it modifies the input data before you have a chance to do validations. For example, it could screw up a date inputand even prevent it from being inserted into the db correctly. Second, it is a generic escape procedure that does not apply to all contexts. There can be differnt procedures for escaping data for different databases - or you might be sending the data to an entirely different system.

 

So, magic quotes - as a rule - should be disabled on the server. But, because a lot of people are on shared servers we do not always have the option to turn it off. So, IF the server has magic quotes turned on THEN you should use strip_tags() ont he input. As I said in the first post, look at the manual for strip tags. There is a snippet of code that you can call in the head of any page that receives user input. IF magic quotes are enabled, strip_tags() will be applied to all of the input values. Plus, if you were to apply strip_tags() when the server is not using magic quotes you can potentially remove some of the user input!

 

That way you will always be dealing with the original input by the user. Then you modify the data as needed based on the context you will use it in.

[Pikachu2000]

Perhaps you mean stripslashes(), rather than strip_tags() . . .

[Psycho]

Perhaps you mean stripslashes(), rather than strip_tags() . . .

 

Ah, sh*t. Yes, you are correct. Anyway, everything I staded previously about strip_tags() still applies.

 

As for strip_tags(), yes you should use that. But, it is debatable about where and how you should use it. There are reasons why a user may use "tags" in their input. For example, someone may put their nickname within their real name such as "Bob <The Boss> Davidson". By using strip_tags() before saving the the database the nickname would be removed without any knowledge of the user.

 

My preference is to always save exactly what was entered. Then when using/displaying the data, escape it as necessay. For example, before saving to the database, use mysql_real_escape_string(). BUt, if I was displaying the data to the page I would use htmlentities() or htmlspecialchars() to ensure the data down not break the HTML code. But, if the data was being used to populate a form field so the value can be edited I would take a different approach.

 

If you escape data in any manner before saving it, you can not restore it (with any confidence) back to its original status.

[chaseman]

Or you could simply allow numbers and the alphabet for the name, if he enters something else he'll get an error message. I find it as user unfriendly if the user enters <bob> and gets registered as bob without any further notification.

 

I will use strip_tags on special occasions and see if I can solve the problem otherwise on other occasions.

 

Thanks for the help, this is a great forum!