Problem
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;
}
?>
Solution
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 (egregisterGuest
). - some of your lines are too long. I would aim for 80 chars max.
!false
is justtrue
🙂- Something like
$securimage->check($_POST['captcha_code']) == !false
can be simplified to$securimage->check($_POST['captcha_code'])
.