0

In the following code in crudindex.php if I enter password with length less than 6 characters error message is not showing using the span command.Required pattern is working. But messages using span command is not displaying ex : if i enter length less than 6 in password no error message displays

What is wrong in this code?

<?php
$con = mysqli_connect("127.0.0.1", "kkits996_ganesh", "", "kkits996_testmysql")  or die("Error " . mysqli_error($con));
$error=false;
if (isset($_POST) && (!empty($_POST))){
 $uname=mysqli_real_escape_string($con,$_POST["uname"]);
 $pwd=mysqli_real_escape_string($con,$_POST["pwd"]);
 $cpwd=mysqli_real_escape_string($con,$_POST["cpwd"]);
$password_error="";
$cpassword_error="";
if(strlen($pwd) < 6) {
        $error = true;
        $password_error = "Password must be minimum of 6 characters";
    }
    if($pwd != $cpwd) {
        $error = true;
        $cpassword_error = "Password and Confirm Password doesn't match";
    }

if (isset($_POST['register'])) {
        # Register-button was clicked


$createsql1="INSERT INTO cruduser(id,username,password) VALUES
                             ('','$uname','$pwd')";

if (mysqli_query($con,$createsql1)) {
echo "Insert Successful in Table cruduser";
mysqli_close($con);
//Redirect because we need to consider the post request from crudadd.php
header( 'Location: crudaddusr.php' ) ;
//include ("crudadd.php");
}
else
{
die(mysqli_error($con));
}
}
if (isset($_POST['login'])) {
        # Login-button was clicked
session_start();
$SESSION['suname']=$uname;
$SESSION['spwd']=$pwd;
if ($uname=='admin' && $pwd=='admin') {
include('crudview.php');
}
else
{
header( "Location: crudeditusr.php?suname=$uname&spwd=$pwd");
}
}
mysqli_close($con);
}
?>
<!--DocType HTML -->
<! bootstrap link is downloaded from bootstrapcdn.com for css and js -->
<! col-mod-6 col-mod-offset are bootstrap related-->
<HTML>
<head>
<title>"Add records in CRUD Table"</title>
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>
</head>
<body>
<div class="container"> 
<div class="row">
<form method="post" class="form-horizontal col-mod-6 col-mod-offset-3">
<h2>Create The table CRUD</h2>
<div class="form-group">
<label for="input" class="col-sm-2 control-label">Username : </label>
<div class="col-sm-10">
<input type="text" name="uname"  required pattern="^[A-Za-z0-9]+" class="form-control" id="input1" placeholder="Username"/>
</div>
</div>
<div class="form-group">
<label for="input" class="col-sm-2 control-label">Password: </label>
<div class="col-sm-10">
<input type="password" name="pwd"  required pattern="^[A-Za-z0-9]+" class="form-control" id="input1" placeholder="Password"/>
<span class="error"><?php if (isset($password_error)) echo $password_error;?></span>
</div>
</div>
<div class="form-group">
<label for="input" class="col-sm-2 control-label">Confirm Password : </label>
<div class="col-sm-10">
<input type="password" name="cpwd"  required pattern="^[A-Za-z0-9]+" class="form-control" id="input1" placeholder="Confirm Password"/>
<span class="text-danger"><?php if (isset($cpassword_error))  echo $cpassword_error; ?></span>
</div>
</div>
<div class="row">

                <div class="col-mod-6 col-mod-offset-3">
                   <button id="submit1" name="register" class="btn btn-primary pull-right">Register</button>
                  <button id="submit2" name="login" class="btn btn-secondary pull-right">Login</button>
                </div>
            </div>
</form>
</body>
</html>
Michael GEDION
  • 879
  • 8
  • 16
yesganesh
  • 35
  • 7
  • 1
    **WARNING**: When using `mysqli` you should be using [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use manual escaping and string interpolation or concatenation to accomplish this because you will create severe [SQL injection bugs](http://bobby-tables.com/). Accidentally unescaped data is a serious risk. Using bound parameters is less verbose and easier to review to check you’re doing it properly. – tadman Aug 15 '17 at 03:42
  • 1
    **WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.4/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and **never store passwords as plain-text**. – tadman Aug 15 '17 at 03:43
  • Note: The object-oriented interface to `mysqli` is significantly less verbose, making code easier to read and audit, and is not easily confused with the obsolete `mysql_query` interface. Before you get too invested in the procedural style it’s worth switching over. Example: `$db = new mysqli(…)` and `$db->prepare("…”)` The only reason the procedural interface exists is because the `mysqli` API was introduced in PHP 4.1 which did not have object-oriented support at the time. – tadman Aug 15 '17 at 03:43
  • What happens when you enter the password less than 6 characters in length? Does it leave the field empty? – Amit Joshi Aug 15 '17 at 03:48
  • No, If I enter char less than 6 its not leaving empty and goes to the next field confirm password. – yesganesh Aug 15 '17 at 03:56
  • Yesganesh, I suggest that you separate registration and login. There is to much code. – Michael GEDION Aug 15 '17 at 04:38

2 Answers2

1

This is a working example to display your errors and prevent some security problems. I have removed the required pattern from your html. You didn't properly set errors. You can handle errors with php and display them. Plus you didn't use action="path/to/handleform.php".

And your redirect should be in login: header( "Location: crudeditusr.php?suname=".$uname."&spwd=".$pwd);

There are 3 security problems here:

  • SQL injection. SOLUTION=> prepared statement
  • Password saved as plain text. SOLUTION=> password_hash()
  • Cross-Site Request Forgery (CSRF). SOLUTION=> input hidden with a token

    <?php
    $con = mysqli_connect("127.0.0.1", "kkits996_ganesh", "", "kkits996_testmysql")  or die("Error " . mysqli_error($con));
    
    // Declare array for errors
    $error=array();
    
    //-----------------------------------------------------//
    //---------------------CSRF PROTECT--------------------//
    //-----------------------------------------------------//
    
    //generate a token/
    function generateToken( $formName )
    {   
        //secret_key change it
        $secretKey ='?@GEskki58668445744!Erpoejsj48';
       if ( !session_id() ) 
       {
           session_start();
       }
       $sessionId = session_id();
       return hash('sha512', $formName.$sessionId.$secretKey );
    }
    
    //check if the token is valid
    function checkToken( $token, $formName)
    {
       return $token === generateToken( $formName );
    }
    
    //Separate REGISTER AND LOGIN TO NOT BE CONFUSED//
    
    //-----------------------------------------------------//
    //---------------------REGISTRATION--------------------//
    //-----------------------------------------------------//
    if ( isset($_POST['register']) && checkToken( $_POST['csrf_token'], 'userFromRegistration' )  ) 
    {
        //if the username required
        if(!preg_match('/^[A-Za-z0-9]+$/',$_POST['uname']))
        {
             $error['username'] = "Username must have alphanumeric characters ";
        }
    
        //if password has less than 6 characters
        if(strlen($_POST['pwd']) < 6)
        {
             $error['password'] = "Password must be minimum of 6 characters";
        }
    
        //if password does not match
       if($_POST['pwd'] !== $_POST['cpwd'] OR empty($_POST['cpwd']) ) 
       {
             $error['passwordmatch'] = "Password and Confirm Password doesn't match";
       }
    
        //if empty error array
        if( !array_filter($error) )
        {
             //trim data
             $username = trim( $_POST['uname'] );
    
             // Hash you password, never save PASSWORD AS PLAIN TEXT!!!!!!!
             // MYSQL! : Allow your storage to expand past 60 characters (VARCHAR 255 would be good)
             $password = password_hash( $_POST['pwd'], PASSWORD_DEFAULT);
    
             //if the id is autoincremented leave id
             //----------USE PREPARED STATEMENT FOR SQL INJECTION---//
    
             $query = 'INSERT INTO cruduser (username, password) VALUES (?,?)';
             $stmt = $con->prepare($query);
             $stmt->bind_param("ss", $username, $password);
             $stmt->execute();
             $stmt->close();
             $con->close();
    
             //Redirect because we need to consider the post request from crudadd.php
             header( 'Location: crudaddusr.php' ) ;
         }
    }
    
    //-----------------------------------------------------//
    //------------------------LOGIN------------------------//
    //-----------------------------------------------------//
    if (isset($_POST['login']))
    {
         //what ever you want
         //Use password_verify() and session_regenerate_id() 
         //to compare passwords and to generate a session id to prevent session fixation.
          session_start();
          $uname = $_POST['uname'];
          $pwd = $_POST['pwd'];
    
          //if you don't need it delete it
          $SESSION['suname']=$unmane;
          $SESSION['spwd']=$pwd;
    
          if ($uname=='admin' && $pwd=='admin')
          {
              include('crudview.php');
          }
          else
          {
              header( "Location: crudeditusr.php?suname=".$uname."&spwd=".$pwd);
          }
        } 
    ?>
    
    <!--HTMl PART-->
    <!DOCTYPE html>
    <html>
        <head>
            <title>"Add records in CRUD Table"</title>
            <!-- bootstrap link is downloaded from bootstrapcdn.com for css and js -->
            <!-- col-mod-6 col-mod-offset are bootstrap related-->
            <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
            <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>
        </head>
        <body>
        <div class="container"> 
            <div class="row">
                <form method="post" action="" class="form-horizontal col-mod-6 col-mod-offset-3">
                <input type="hidden" name="csrf_token" value="<?php echo generateToken('userFromRegistration'); ?>" required/>
                <h2>Create The table CRUD</h2>
                <div class="form-group">
                    <label for="input" class="col-sm-2 control-label">Username : </label>
                    <div class="col-sm-10 <?php if( !empty( $error['username'] ) ){ echo 'has-error';}  ?> ">
                        <input type="text" name="uname" class="form-control" id="input1" placeholder="Username"/>
                        <span class="help-block"><?php if (!empty($error['username'])) echo $error['username'];?></span>
                    </div>
                </div>
                <div class="form-group">
                    <label for="input" class="col-sm-2 control-label">Password: </label>
                    <div class="col-sm-10 <?php if( !empty( $error['password'] ) ){ echo 'has-error';}  ?>">
                        <input type="password" name="pwd"  class="form-control" id="input1" placeholder="Password"/>
                        <span class="help-block"><?php if (!empty($error['password'])) echo $error['password'];?></span>
                    </div>
                </div>
                <div class="form-group">
                    <label for="input" class="col-sm-2 control-label">Confirm Password : </label>
                    <div class="col-sm-10 <?php if( !empty( $error['passwordmatch'] ) ){ echo 'has-error';}  ?>">
                        <input type="password" name="cpwd" class="form-control" id="input1" placeholder="Confirm Password"/>
                        <span class="help-block"><?php if (!empty($error['passwordmatch'])) echo $error['passwordmatch'];?></span>
                    </div>
                </div>
                <div class="row">
    
                    <div class="col-mod-6 col-mod-offset-3">
                       <button id="submit1" name="register" class="btn btn-primary pull-right">Register</button>
                       <button id="submit2" name="login" class="btn btn-secondary pull-right">Login</button>
                   </div>
               </div>
           </form>
       </body>
    

Michael GEDION
  • 879
  • 8
  • 16
  • Fine. Registrtion is fine and table insert is fine. But When i made the changes in the Login and after redirecting the crudeditusr.php its not populating the data for the selected user and pwd.header( "Location: crudeditusr.php?suname=$uname&spwd=$pwd"); In crudeditusr.php I use get command $uname=$_GET['suname']; $pwd=$_GET['spwd']; – yesganesh Aug 15 '17 at 10:19
  • I have update the answer, please l=take a look the login part: your header will not work. It must be `header("Location: crudeditusr.php?suname=".$uname."&spwd=".$pwd);`. The in the In crudeditusr.php, use `$uname=$_GET['suname']; $pwd=$_GET['spwd']; echo $uname; echo $pwd;` Let me know if it works. – Michael GEDION Aug 15 '17 at 12:06
  • I will try. Also when the the password using $_GET will be normal string. But in the table its stored as hash. How to compare the string and hash . Now I am checking this. – yesganesh Aug 15 '17 at 12:30
  • username and pwd is correct. But the id is not getting pickedup. $uname=$_GET['suname']; $pwd=$_GET['spwd']; $selsql = "SELECT * FROM cruduser WHERE username='$uname'"; $res = mysqli_query($con, $selsql); $count = mysqli_num_rows($res); if($count == 1){ $row = mysqli_fetch_array($res); print "hashedPass = ${row['password']}"; print "myPassword: " . $pwd; if(password_verify($pwd, $row['password'])){ print "Password match"; $id=$row['id']; } else print "The username or password do not match"; } – yesganesh Aug 15 '17 at 15:16
  • ` $uname=$_GET['suname'];$pwd=$_GET['spwd']; $selsql = "SELECT * FROM cruduser WHERE username='$uname'";$res = mysqli_query($con, $selsql); $row = mysqli_fetch_array($res); $count = mysqli_num_rows($res); if($count > 1){ echo "hashedPass :". $row['password'] .""; echo "myPassword :" . $pwd ."";if(password_verify($pwd, $row['password'])){echo "Password match".""; $id=$row['id']; } else{ echo "The username or password do not match"."";}` $count is $count>0. improved code. – Michael GEDION Aug 15 '17 at 15:55
  • No luck. Still "The username or password do not match". Any clue? – yesganesh Aug 15 '17 at 16:38
  • Test with a new password, this should work! I have tested it, it works. Anyways if my post helped you and was usefull please vote up and accept it as an accepted answer :). – Michael GEDION Aug 15 '17 at 16:52
  • just 2 doubts. 1. In the crudindex.php if anyone login with admin and pwd adminx it should open crudview.php. When i made the changes in code all the users pressing the login button it goes to crudview.php. Also the echo values are not showing. 2. why i need to regenarate session.id for login screen? code for 1 is below --//------------------------LOGIN as admin---------------------// if ( isset($_POST['login'])) { if ($_POST['uname']="admin" && $_POST['pwd']="adminx") { echo $_POST['uname']; echo $_POST['pwd']; $con->close(); include("crudview.php"); } } – yesganesh Aug 16 '17 at 07:05
  • It is better that you ask another question.. post all your code and will see it and see what is not working. – Michael GEDION Aug 16 '17 at 07:07
  • I asked another ?. subject : Redirection not working in php. – yesganesh Aug 16 '17 at 07:50
  • where is the link – Michael GEDION Aug 16 '17 at 08:22
  • //------------------------LOGIN as admin---------------------// if ( isset($_POST['login'])) { if ($_POST['uname']=="admin" && $_POST['pwd']=="adminx") { echo $_POST['uname']; echo $_POST['pwd']; $con->close(); header ("Location: crudview.php"); } } – yesganesh Aug 16 '17 at 08:26
0

Change this line from this,

$pwd=mysqli_real_escape_string($con,$_POST["pwd"]);

to this,

$pwd=mysqli_real_escape_string($_POST["pwd"]);

You don't need to add $con to assign your password to another variable but it's better use the direct $_POST variable no need to assign it to another.

if(strlen($_POST["pwd"]) < 6) {
        $error = true;
        $password_error = "Password must be minimum of 6 characters";
    }
S4NDM4N
  • 904
  • 2
  • 11
  • 26
  • Did you try just to `echo` the message with out the `if` condition? if not try it and let me know. Or add a else part to your `if` where you try to `echo` error if the `else` part executes then that means it's not setting your password variable. – S4NDM4N Aug 15 '17 at 05:24
  • echo outside if is working . echo inside if not working, Inside this statement if ((isset($_POST)) && (!empty($_POST))) its not working. But data is getting inserted which is inside the if statement.. Data is inserted with entered username, password. – yesganesh Aug 15 '17 at 06:17