Registration Form - Lookover

Just wanting someone to look over my script

Page 1 of 1

9 Replies - 984 Views - Last Post: 13 April 2010 - 01:54 AM Rate Topic: ***** 1 Votes

#1 Shukumei  Icon User is offline

  • D.I.C Head

Reputation: 2
  • View blog
  • Posts: 80
  • Joined: 22-September 08

Registration Form - Lookover

Posted 09 April 2010 - 09:01 PM

Hi all,

Just wondering if someone can look over my script for a registration form - Just want to know a. can it be refined(my guess is yes) b. have I missed any validations that a registration form should have and c. anything you may consider important.

Thank you in advance.

Here is the reg.php:
<?php
    //Includes.
    include "functions/dbconnection.php";  //Access database connection.
    include"functions/functions.php";  //Allows access to various functions found in the functions.php file.

    //Get client's contact details from form.    
    $firstName = secure($_POST['firstName']);
    $lastName = secure($_POST['lastName']);
    $phoneNumber = secure($_POST['phoneNumber']);
    $emailAddress = secure($_POST['emailAddress']);
    $street = secure($_POST['street']);
    $suburb = secure($_POST['suburb']);
    $city = secure($_POST['city']);
    $state = secure($_POST['state']);
    $postcode = secure($_POST['postcode']);

    //Get client's desired login details from form.        
    $userName = secure($_POST['userName']);
    $password = secure($_POST['password']);
    $passwordReEnter = secure($_POST['passwordReEnter']); 

    //Check to see if fields are left blank
    if(empty($firstName)||empty($lastName)||empty($phoneNumber)||empty($emailAddress)
    ||empty($street)||empty($suburb)||empty($city)||empty($state)||empty($postcode)
    ||empty($userName)||empty($password)||empty($passwordReEnter))
    {  
        $error = 'Please fill in all fields.';     
    }else{
        //Check to if user entered correct password.
        if($password != $passwordReEnter){
            $error = 'Passwords Must Match.';
        }else{
            //Check if phonenumber and postcode are numeric
            if(!is_numeric($phoneNumber)||!is_numeric($postcode)){
                $error = 'Phone and Postcode must be numeric';
            }else{
                //Check lengths of phonenumber and postcode.
                if(strlen($phoneNumber)!= 10 || strlen($postcode)!= 4){
                    $error = 'Phone number must be 10 digits long and postcode must be 4 digits long.';

                }else{
                    //Email Validation                
                    if($emailAddress != filter_var($emailAddress, FILTER_VALIDATE_EMAIL))
                    {                           
                        //If email validation fails (returns false) Cancel the opperation.
                        die($emailAddress." is an invalid email address.");       
                    }else{
                        //Insert user data into user table.
                        $addClient="INSERT INTO client (firstName, lastName, phoneNumber, emailAddress, street, suburb, city, state, postcode)
                        VALUES ('$firstName', '$lastName', '$phoneNumber', '$emailAddress', '$street', '$suburb', '$city', '$state', '$postcode')";

                        if(@mysql_query($addClient))
                        {
                            //If the client submits a user name.
                            if(isset($userName))
                            {
                                //Get user ID (clientID(FK))
                                $clientID=mysql_insert_id();

                                //Insert the user details.
                                $addUser="INSERT INTO user (userID, userName, password, clientID)
                                VALUES (NULL, '$userName', '$password', $clientID)"; 
                                $addResult=mysql_query($addUser);
                            }

                            // Display first name and user name - Used to check if client was added correctly.
                            echo "Thank you ".$firstName." your user name is: ".$userName."<br>";  
                            echo("<p>Client Added.</p>");
                        }    
                        else{
                            //Display insertion error and mysql specific error message.
                            echo("<p>Error adding client: </p>".mysql_error()."</p>");
                        }
                    }         
                }
            }
        }
    }
    echo $error;  
?>
<html>
    <body>
        <a href="index.php">Login</a>
    </body>
</html>


Here is the function for 'secure' from functions.php:
//Prevents SQL injection by trimming than escaping the inputed varible.
    function secure($input){
        $input = mysql_real_escape_string(trim($input));
        return $input;
    }


And last here is the database connection script from dbconnection.php:
<?php
    //Set strings for host, user, password and database name for access to
    //the database.
    $dbHost='localhost'; //The host name - for the time being localhost will do the job.
    $dbUser ='root'; //The name of user accessing the database - root normally has no restrictions.
    $dbPassword='******'; //Use '' if no password is set. Must be the matching user password.
    $dbName='DainDev_DB'; //Can not be NULL - you must enter a valid database name.
    
    //Connect to the database server.
    $dbCon = @mysql_connect($dbHost, $dbUser, $dbPassword);
        if(!$dbCon){
            echo("<p>Cannot conect to the database server.</p>");
            exit();
        }
    
    //Connect to the database.
    mysql_select_db($dbName, $dbCon);
    if(!@mysql_select_db($dbName)){
        echo("Cannot locate ".$dbName." database.</p>");
        exit();
    }
?>


Thank you again to anyone who looks over this for me.

Is This A Good Question/Topic? 0
  • +

Replies To: Registration Form - Lookover

#2 alienDeveloper  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 27
  • Joined: 11-November 08

Re: Registration Form - Lookover

Posted 09 April 2010 - 09:24 PM

"Dont store password in plain text". use md5 or sha1 encrytion atleast. For added securty i suggest adding a password salt to it and then md5 it. And if you are following my suggestion den you don't have to use secure() function for password.

Just my 2 cents.
Was This Post Helpful? 1
  • +
  • -

#3 Shukumei  Icon User is offline

  • D.I.C Head

Reputation: 2
  • View blog
  • Posts: 80
  • Joined: 22-September 08

Re: Registration Form - Lookover

Posted 09 April 2010 - 09:32 PM

I knew I would forget something important - I had read about md5 a while back and thought to myself I must remember that so can read up on it later. Thanks for the reminder.

Also - what is a "password salt"?

One more thing I get the whole having to be secure with passwords, but will md5 and the 'salt' thing prevent SQL injection from the password input fields? Cos that is what my 'secure' function is meant for (maybe 'secure' was a bad name for the function it is not very descriptive).

Thank you for you '2 Cents'
Was This Post Helpful? 0
  • +
  • -

#4 CTphpnwb  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 2891
  • View blog
  • Posts: 10,025
  • Joined: 08-August 08

Re: Registration Form - Lookover

Posted 09 April 2010 - 10:01 PM

Using a salt makes it harder to crack a password because the hash is harder to "lookup." You might generate the salt by using known information about the user such as their user name or you might use a constant only you know, or a combination of both:

$hashed_password = md5($password.$username."myconstant");


This way, two users with the same password will generate different hashes, so a hacker will have trouble using a hash to gain entry.
Was This Post Helpful? 1
  • +
  • -

#5 Shukumei  Icon User is offline

  • D.I.C Head

Reputation: 2
  • View blog
  • Posts: 80
  • Joined: 22-September 08

Re: Registration Form - Lookover

Posted 09 April 2010 - 10:27 PM

View PostCTphpnwb, on 09 April 2010 - 09:01 PM, said:

Using a salt makes it harder to crack a password because the hash is harder to "lookup." You might generate the salt by using known information about the user such as their user name or you might use a constant only you know, or a combination of both:

$hashed_password = md5($password.$username."myconstant");


This way, two users with the same password will generate different hashes, so a hacker will have trouble using a hash to gain entry.


Oh wow, that is really awesome. Salt and md5 is what I am going to then. Wow that is awesome... sorry sometime I get over excited.

I assume the rest of my script is ok then?
Was This Post Helpful? 0
  • +
  • -

#6 alienDeveloper  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 27
  • Joined: 11-November 08

Re: Registration Form - Lookover

Posted 09 April 2010 - 10:55 PM

i hope u understant the password salt part.
And abt sql injection. whn u md5 .. u will get a 35 charater hex decimal ( 0-9A-F ) string which is ofcourse safe to insert directly into db.


and one more performance tip i like to give is replace empty with isset () ;;
for example to check username
insted of

empty ( $userName ) .. use .. isset ( $userName{0} ) .. or may be isset ($userName{1} ) ..

Another problem with empty is your user wont be able to give username or password as 00000 or false which are suppose to be valid.

This post has been edited by alienDeveloper: 09 April 2010 - 10:56 PM

Was This Post Helpful? 1
  • +
  • -

#7 Guest_grimpirate*


Reputation:

Re: Registration Form - Lookover

Posted 09 April 2010 - 11:07 PM

This is only an aesthetic suggestion to your code. As opposed to using a series of nested elses and ifs, why not instead use elseif statements to make your code a little more read friendly?
Was This Post Helpful? 1

#8 Shukumei  Icon User is offline

  • D.I.C Head

Reputation: 2
  • View blog
  • Posts: 80
  • Joined: 22-September 08

Re: Registration Form - Lookover

Posted 10 April 2010 - 12:05 AM

Thank you alienDeveloper I never thought of the whole 00000 or false thing, also now you point it out to me I see how the sql injection is counteracted with md5 - Thank you.

And thank you Guest_grimpirate, yes you are right - I will work on that as soon as possile, I love code that is easy to read - so when I can I love to make code that is easy for others to read - Thank you
Was This Post Helpful? 0
  • +
  • -

#9 jrm402  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 51
  • View blog
  • Posts: 333
  • Joined: 18-March 10

Re: Registration Form - Lookover

Posted 12 April 2010 - 08:49 AM

I don't know if you did this but on the form side of things I generally do something like this:
<input type="text" name="firstName" value="<?php echo $_POST['firstName']; ?>" />



This is only good if your working on the same page. It is a nice feature so the user doesn't have to re-enter all of the data if they did something wrong.
Was This Post Helpful? 0
  • +
  • -

#10 alienDeveloper  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 27
  • Joined: 11-November 08

Re: Registration Form - Lookover

Posted 13 April 2010 - 01:54 AM

Something i just thought about.

have you checked wheather the username and email already exists in the database. i think its a pretty big issue because i cant find a code that does it in this one.

also in db file do u really need
    mysql_select_db($dbName, $dbCon);
    if(!@mysql_select_db($dbName)){


i think u only need the second line

This post has been edited by alienDeveloper: 13 April 2010 - 02:05 AM

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1