Basic registration page and database insertion

Question

Please review my registration code and suggest what changes I should make to improve security. I'm new to PHP and this is my first project. Also I have no Idea how to make a "forget password page" as I've used password_hash to store passwords in database.

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>VALIDATION FORM - REGISTER</title>
<link href="style.css" rel="stylesheet" type="text/css">
<script src="js/gen_validatorv4.js" type="text/javascript"></script>
</head>

<body>

<?php
<!--FILE TO CONNECT TO DATABASE AND INSERTION-->
include 'register_db_check.php';
?>

<!--PHP FLE TO CHECK VALIDATION-->
<?php include 'register_validation.php';?>

<h2>DomainName.com - REGISTER</h2>

<!--NAVBAR-->
<div>
<nav class="navBar"><a href="login.php"> Login </a> | <a    href="register.php"> Register </a>  |  <a href="#"> Contact Us </a></nav>
</div>

<!--FORM STARTS HERE-->
<form name="myForm" id="form" enctype="multipart/form-data" method="post" action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]);?>">
<table>
<tr>

<!--USERNAME FIELD-->
<td>User Name :</td> 
<td>
<input type="text" placeholder="Enter User Name Here" name="uname" id="uname" size="35" value="<?php echo $uname;?>">
<span class="error">* <?php echo $unameErr;?>
</span>
</td>
</tr>

<!--NAME FIELD-->
<tr>
<td>Name :</td> 
<td><input type="text" placeholder="Enter Name Here" name="name" id="name" size="35" value="<?php echo $name;?>">
<span class="error">* <?php echo $nameErr;?></span>
</td>
</tr>

<!--FNAME FIELD-->
<tr>
<td>Father Name : </td>
<td><input type="text" placeholder="Enter Father Name Here" name="fname" id="fname" size="35" value="<?php echo $fname;?>">
<span class="error">* <?php echo $fnameErr; ?></span>
</td>
</tr>

<!--EMAIL FIELD-->
<tr>
<td>E-mail : </td>
<td><input type="text" name="email" id="email" placeholder="Enter Valid E-mail Here" size="35" value="<?php echo $email;?>">
<span class="error">* <?php echo $emailErr; ?></span>
</td>
</tr>

<!--PASSWORD FIELD-->
<tr>
<td>Password :</td> 
<td><input type="password" placeholder="Enter Password Here" name="password" id="password"size="35" value="<?php echo $pass;?>">
<span class="error">* <?php echo $passErr; ?></span>
</td>
</tr>

<!--MOBILE NUMBER-->
<tr>
<td>Mobile Number :</td> 
<td><input type="tel" placeholder="Enter Valid PH No. Here" name="mnum" id="mnum" size="35" value="<?php echo $mnum;?>">
<span class="error">* <?php echo $mnumErr; ?></span>
</td>
</tr>

<!--INTEREST FIELD-->
<tr>
<td>Interests :</td>
<td><textarea name="interest" id="interest" cols="40" rows="5"><?php echo $intrs;?></textarea><span class="error">* <?php echo $intrsErr;?>
</span>  
</td>
</tr>

<!--RADIO BUTTON-->
<tr>
<td>Gender: </td>
<td>
<input type="radio" name="sex" value="Male" <?php if(isset($sex) && $sex == "Male") echo "Checked";?> >Male

<input type="radio" name="sex" value="Female" <?php if(isset($sex) && $sex == "Female") echo "Checked";?> >Female 
<span class="error">* <?php //echo $sexErr; ?></span> </td>
</tr>


<!--Captcha Image!-->
<tr><td></td>
<td><img id="captcha" src="/securimage/securimage_show.php" alt="CAPTCHA Image"/></td>
<tr><td>Enter Captcha Here: </td><td><input type="text" name="captcha_code" size="10" maxlength="6" >

</td></tr>
</tr>
<tr><td><a href="#" onclick="document.getElementById('captcha').src = '/securimage/securimage_show.php?' + Math.random(); return false"></a>
</td></tr>

<!--REGISTER BUTTON-->
<tr>
<td></td>
<td>
<input type="submit" value="Register" name="reg" class="regbtn" ></td>
</tr>

<tr><td></td>
<!--OR BUTTON-->
<td><h3>OR</h3></td></tr>
<tr><td></td>

<!--SIGN-IN BUTTON-->
<td><h3>Click Below To Sign In From Your ID</h3></td>
</tr>

<tr><td></td>
<td><a href="login.php"><input type="button" value="Sign-in" name="button" id="sbtn"></a></td>
</tr>

</table>

</form>

<!--FORM VALIDATION JAVASCRIPT-->
<script type="text/javascript">
    var frmvalidator = new Validator("myForm");
    frmvalidator.addValidation("uname","req","Please enter a Valid User Name");
    frmvalidator.addValidation("name","req","Please enter your First Name");
    frmvalidator.addValidation("fname","req","Please enter your Father Name");

    frmvalidator.addValidation("email","req","Please enter your Email");
    frmvalidator.addValidation("email","email","Please enter Valid Email");

    frmvalidator.addValidation("mnum","numeric","Only digits are allowed");

    chktestValidator.addValidation("sex","selone_radio","Select atleast one");
</script>

<div class="footer">
<?php include('footer.php');?>
</div>

</body>
</html>

register_db_check.php

<?php 
include_once $_SERVER['DOCUMENT_ROOT'] . '/securimage/securimage.php';

    $securimage = new Securimage();
?>
<?php 
    require('connect.php');
    // If the values are posted, insert them into the database.
    if (isset($_POST['uname']) && isset($_POST['password']) && isset($_POST['fname']) && isset($_POST['email'])
     && isset($_POST['password']) && isset($_POST['mnum']) && isset($_POST['interest']) && !empty($_POST['mnum'])) 
    {

         $uname = $_POST['uname'];
         $name = $_POST['name'];
         $fname = $_POST['fname'];
         $email = $_POST['email'];
         $pass = password_hash($_POST['password'], PASSWORD_DEFAULT);
         $mnum = $_POST['mnum'];
         $intrs = $_POST['interest'];


        $sql = mysqli_query($conn, "SELECT * FROM `guests` WHERE email = '".$email."'");
            if(mysqli_num_rows($sql) > 0) {
                header ("Location:register.php?error=Email already exist.");
                exit();
                }
        $sql = mysqli_query($conn, "SELECT * FROM `guests` WHERE uname = '".$uname."'");
            if(mysqli_num_rows($sql) > 0) {
                /*print '<script>alert("Username already In Use!");</script>'; //Prompts the user
                print '<script>window.location.assign("register.php");</script>';*/  //redirects to login.php
                header ("Location:register.php?error=Username already exist.");
                exit(); 
                }


        $result = mysqli_query($conn, "INSERT INTO `guests`(uname, name, fname, email, password, mnum, interest) VALUES                         ('$uname','$name','$fname','$email','$pass','$mnum', '$intrs')") or die(mysqli_connect_error());


        if($result == TRUE && isset($_POST['captcha_code']) && $securimage->check($_POST['captcha_code']) == !false) {

            header('Location:/FvWithCaptcha/thankyou.php');
        }
                elseif ($result == FALSE || $result == NULL || $securimage->check($_POST['captcha_code']) == false) {
                    echo "Error in registration";
            }

mysqli_close($conn);                    
?>

Any tutorials are also appreciated!

register_validation.php

<?php

$unameErr = $nameErr = $fnameErr = $emailErr = $passErr = $mnumErr = $sexErr = $intrsErr =  "" ;

$uname =$name = $fname = $email = $pass = $mnum = $sex = $intrs =  "" ;

if ($_SERVER["REQUEST_METHOD"] == "POST") {

    if(empty($_POST["uname"])) {
        $unameErr = "User Name Is Required";
        }
            else
            {
                $uname = test_input($_POST["uname"]);
            }

    if(empty($_POST["name"])) {
        $nameErr = "Name Is Required";
        }
            else
            {
                $name = test_input($_POST["name"]);

            //Condition Check
            if(!preg_match("/^[a-zA-Z ]*$/", $name)){
                $nameErr = "Only Letters are allowed";
                }   
            }

    if(empty($_POST["fname"])){
        $fnameErr = "Father Name Is Required";
        }
            else
            {
                $fname = test_input($_POST["fname"]);
                //Condition Check
            if(!preg_match("/^[a-zA-Z ]*$/", $fname)){
                $fnameErr = "Only Letters are allowed";}
                }
    if(empty($_POST["email"])){
        $emailErr = "Email Required";
        }
            else {
                $email = test_input($_POST["email"]);
                //Condition Check
                if(!filter_var($email, FILTER_VALIDATE_EMAIL)) {
                    $emailErr = "Enter Valid E-mail";
                    }   
                }


    if(empty($_POST["mnum"])){
        $mnumErr = "PH No. Required";
        }
            else {
                $mnum = test_input($_POST["mnum"]);
                if(preg_match("/[^0-9]/", $mnum))
                    {
                $mnumErr = "Only Digits are allowed";
                    }
                }


    if(empty($_POST["password"])){
        $passErr = "Password Required";
        }
            else {
                $pass = test_input($_POST["password"]);
                }           

    if(empty($_POST["sex"])){
        $sexErr = "Gender is Required";
        }
            else {
                $sex = test_input($_POST["sex"]);
                }

    if(empty($_POST["interest"])) {
        $intrsErr = "Show some Interest please!";

        }
            else {
                $intrs = test_input($_POST["interest"]);
                }


    }


function test_input($data) {
    $data = trim($data);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    return $data;
    }

?>

tim · Accepted Answer · 2015-06-05 10:45:36Z

Security

SQL Injection

Never put any variable data directly into SQL queries (the type of query doesn't matter, all can be vulnerable to SQL injection). This allows attackers to execute SQL queries themselves, with which they can see confidential database data. Depending on the db software and settings, they might also be able to write to files (which might gain them code execution), read sensitive files, execute system commands, etc.

Always use prepared statements.

XSS

Never echo variable data without sanitizing it. You do a good job here with htmlspecialchars($_SERVER["PHP_SELF"]), but not with the rest of the variables (eg $uname, $name, etc, all of which are also user supplied). This allows an attacker to execute arbitrary JavaScript in the context of the victims browser, which allows the stealing of cookies, key logging, phishing, and bypassing of CSRF (which means the attacker can do anything the victim can do).

Always use htmlspecialchars($string, ENT_QUOTES, 'UTF-8') whenever echoing a variable. Also note this list of places where this in not enough to prevent XSS.

Passwords

password_hash is the correct way of handling this. Regarding your question: A forgot password feature should NOT be sending the plaintext password to the user. Instead, a password reset link with a token which can be validated should be send to the users email address.

Captcha

You seem to validate the captcha after inserting the user into the database. This doesn't make any sense. The only thing the captcha protects is the redirection to /FvWithCaptcha/thankyou.php. But a user could just enter that path on their own.

Misc

your coding style makes it hard to read your code. Use proper indentation and be consistent with the location of your curly brackets. Any IDE will do this for you.
comments that just repeat what the code says are not helpful (eg If the values are posted, insert them into the database.). If you think that it's not obvious that this block of code does that, extract it to a well-named function (eg registerGuest).
some of your lines are too long. I would aim for 80 chars max.
!false is just true :)
Something like $securimage->check($_POST['captcha_code']) == !false can be simplified to $securimage->check($_POST['captcha_code']).

I really appreciate your help. I totally get your SQL Injection, passwords and MISC part. For XSS I wanna know if my register_validation.php file is good enough? Also I wanna know how can I improve my Captcha Code part? — zer0_c00l, Jun 5 '15 at 12:20
@zer0_c00l you didn't post your register_validation.php file. But my guess would be no (and even if it is good enough, you should really defend against XSS directly when echoing, not in some other file far away). Regarding the captcha: You should check it before inserting the user into the db. Because that is what it should protect, right? — tim, Jun 5 '15 at 12:26
I just posted the file. I'll try to improve my captcha and update you:) — zer0_c00l, Jun 5 '15 at 12:29
In this case, register_validation.php is good enough. But this really isn't the recommended approach. What if you misplace that file? Your script would still work, but be vulnerable. What if you add another field to register_db_check.php and register.php, but forget to add it to register_validation.php? Again, your script would be vulnerable. Also, ENT_QUOTES for htmlentities is recommended, because if you ever would do something like this: echo "<input type='text' name='uname' value='" . test_input($_POST['uname']) . "'>"; that would be vulnerable without it. — tim, Jun 5 '15 at 12:42
@zer0_c00l Also, if you use stripslashes, you should check if magic quotes are enabled. Because that's the only reason to use stripslashes: to remove the slashes added by magic quotes. If you don't do this, a user could use a password like foo\'bar, make an error in some other field, submit, correct that error and not change their password, submit, and now not be able to log in with foo\'bar, but only with foo'bar. — tim, Jun 5 '15 at 12:45

Stack Exchange Network

current community

your communities

more stack exchange communities

Basic registration page and database insertion

1 Answer 1

Security

Misc

Your Answer

Not the answer you're looking for? Browse other questions tagged php security mysqli or ask your own question.

Hot Network Questions

Basic registration page and database insertion

1 Answer 1

Security

Misc

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged php security mysqli or ask your own question.

Related

Hot Network Questions