1

I am learning how to use prepared statements in my simple login system to make it more secure.

I have followed a few different tutorials to get it working but cant get it to work. When i enter the username and password wrong it gives me the error. When i enter the username and password correct i still get the error.

What am i doing wrong?

I am new to this so apologies for any obvious errors.

I have also looked into hashing my password as it is being stored as plain text in the database at the moment but that will be my next step after i get this working.

Here is my code:

<?php

error_reporting(E_ALL); ini_set('display_errors', 1);
session_start(); // Starting Session
$error=''; // Variable To Store Error Message

if($SERVER['REQUESTMETHOD'] == 'POST') {
if (empty($POST['username']) || empty($POST['password'])) {

$error = "Enter Username and Password";

}

else
{
// Define $username and $password
$username = $_POST['username'];
$password = $_POST['password'];

//connect to database
include('dbconx.php');

}

$stmt = $con->prepare("SELECT * from admin where password=? AND username=?");
$stmt->bind_param('ss', $username, $password); 
$stmt->execute();
$stmt->bind_result($id, $username, $password);
$stmt->store_result();
if($stmt->num_rows == 1) //To check if the row exists
{

        $_SESSION['login_user'] = $username; // Initializing Session
        header("location: confirm.php"); // Redirecting To Other Page

    }
else {
$error = "Username or Password is incorrect";
}

mysqli_close($con); // Closing Connection
}

?>
Danni1990
  • 21
  • 3
  • What is the "error"? – chris85 Mar 28 '18 at 23:19
  • Never use `SELECT *` with `bind_result()`. You need to explicitly set the order, ie `SELECT id, username, password`. That being said, there's no need to `SELECT` the password or username – Phil Mar 28 '18 at 23:20
  • 1
    Oh, you have your arguments backwards. Your query binds `password` then `username` but your `bind_param()` uses `$username` then `$password` – Phil Mar 28 '18 at 23:22
  • The error is just stating that username or password is wrong.... – Danni1990 Mar 29 '18 at 19:20

3 Answers3

2

You have your bound parameter arguments backwards. Your query binds password then username but your bind_param() uses $username then $password.


I've never been a fan of using the number of rows returned to determine existence. Instead, you can simply use fetch(). It will return a boolean value indicating whether or not there was a result.

For example

$stmt = $con->prepare('SELECT id from admin where password = ? AND username = ?');
$stmt->bind_param('ss', $password, $username); // note the order
$stmt->execute();
$stmt->bind_result($id);
if ($stmt->fetch()) {
    $_SESSION['login_user'] = $username;
    $_SESSION['login_user_id'] = $id; // probably important
    header("Location: confirm.php");
    exit; // always exit after a "Location" header
} else {
    $error = "Username or Password is incorrect";
}
Phil
  • 157,677
  • 23
  • 242
  • 245
  • Thanks for the advice Phil. I have changed my code a little and still having the same problem. No matter what i enter into username and password fields the error still appears and does not redirect me to my confirm page. Any suggestions? – Danni1990 Mar 29 '18 at 19:13
  • I managed to fix it by changing to code a bit to this: $stmt = $con->prepare('SELECT * FROM admin WHERE username = ? AND password = ?'); $stmt->bind_param('ss', $username, $password); $stmt->execute(); $stmt->bind_result($id, $username, $password); – Danni1990 Mar 29 '18 at 19:59
-1

mysqli_stmt::store_result should be called before mysqli_stmt::bind_result, also you would need to call mysqli_stmt::seek_data and mysqli_stmt::fetch to get the result.

Example :

<?php

$db = new Mysqli(...);

$inputUsername = $_POST['username'] ?? '';
$inputPassword = $_POST['password'] ?? '';

$statment = $db->prepare('SELECT `id`,`username`,`password` FROM `admin` WHERE `username` = ?');

$statment->bind_param('s',$inputUsername);

$statment->execute();

$statment->store_result();

$statment->bind_result($id,$username,$password);

if($statment->num_rows) {

    $statment->data_seek(0);
    $statment->fetch();

    /**
    * this is not secure
    * @see http://php.net/manual/en/function.password-hash.php
    */
    if($inputPassword === $password) {
        echo sprintf('Welcome, %s!',$username);
    } else {
        echo 'Incorrect password!';
    }

} else {

    echo sprintf('No such user with the given username (%s)',$inputUsername);

}

$statment->close();
$db->close();
azjezz
  • 3,827
  • 1
  • 14
  • 35
-1

Removed bind_result and store_result for get_result and fetch_assoc. It makes getting db records more flexible and stable.

Also added exit(); after redirection so no other codes will be executed after redirect command.

Typo in:

if (empty($POST['username']) || empty($POST['password']))

  • ^ $POST should be $_POST instead.

$error is not being checked properly if empty or not. And still goes through mysqli functions block even if there is an error. Fixed that by creating an appropriate if statement that encloses the mysqli funtions block.

Also added proper indentation to the code for readability.

New Code:

<?php
error_reporting(E_ALL); 
ini_set('display_errors', 1);
session_start(); // Starting Session
$error=''; // Variable To Store Error Message

$_POST['username'] = isset( $_POST['username'] ) ? $_POST['username'] : '';
$_POST['password'] = isset( $_POST['password'] ) ? $_POST['password'] : '';

if($_SERVER['REQUEST_METHOD'] == 'POST') {
    if (empty($_POST['username']) || empty($_POST['password'])) {
        $error = "Enter Username and Password";
    }
    else{
        // Define $username and $password
        $username = $_POST['username'];
        $password = $_POST['password'];

        //connect to database
        include('dbconx.php');
    }

    if( $error == "" ) {
        $stmt = $con->prepare("SELECT * from students where username=? AND password=?");
        $stmt->bind_param('ss', $username, $password); 
        $stmt->execute();
        $result = $stmt->get_result();

        if($result->num_rows == 1) {
            $row = $result->fetch_assoc();
            $_SESSION['login_user'] = $row['username']; // Initializing Session
            header("location: confirm.php");exit(); // Redirecting To Other Page
        }
        else {
            $error = "Username or Password is incorrect";
        }

        mysqli_close($con); // Closing Connection
    }
    echo $error;
}

?>
Karlo Kokkak
  • 3,674
  • 4
  • 18
  • 33