1

I'm having a very annoying problem with a very simple PHP login script. The problem is that the login works even with wrong username and/or password. I'm running it in XAMPP 7.0.8 for Linux. By the way, the table "users" of the database "login" has only one row.

I've been trying to understand how these kind of scripts work, so I tried with the most basic kind of login I could. But it keeps being a mystery to me, by now I've never could make it work it out.

Here is the PHP part:

<?php

    session_start();

    if(isset($_POST['login'])) {


        $db = mysqli_connect("localhost", "root", "", "login");

        $username = strip_tags($_POST['username']);
        $password = strip_tags($_POST['password']);

        $username = stripcslashes($username);
        $password = stripcslashes($password);

        $username = mysqli_real_escape_string($username);
        $password = mysqli_real_escape_string($password);

        $sql = "SELECT * FROM users WHERE username = '$username' LIMIT 1";
        $query = mysqli_query($db, $sql);
        $row = mysqli_fetch_array($query);
        $id = $row['id'];
        $db_password = $row['password'];

        if($password == $db_password){
            $_SESSION['username'] = $username;
            $_SESSION['id'] = $id;
            header("Location: index.php");
        } else {
            echo "Error: the information is not correct.";
        }


    }
?>

And here the HTML form:

<!DOCTYPE html>
    <html lang="es">
    <head>
        <meta charset="UTF-8">
        <title>Client's area</title>
    </head>
    <body>
            <form method="post" action="">
                    <input type="text" name="username">
                    <input type="password" name="password">
                    <input class="login" type="submit" name="login" value="Login">
            </form>
    </body>
    </html> 

Edit: the idea of this post was never trying to make a secure login script, it was intended to understand some PHP features. I recognize it's not safe to use any part of this code for an actual login project.

sebasaenz
  • 1,917
  • 2
  • 20
  • 25
  • 1
    please dont store plain text password in your database. Use password hashing. PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) – RiggsFolly Aug 31 '16 at 17:28
  • 1
    Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Aug 31 '16 at 17:29
  • 2
    This is a really bad login system, you are storing passwords as plain text, please go out and find a different tutorial to follow that uses PDO and `password_hash` http://php.net/manual/en/function.password-hash.php – cmorrissey Aug 31 '16 at 17:29
  • 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.2/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and **never store passwords as plain-text**. Everything about this code is wrong. – tadman Aug 31 '16 at 17:31
  • 1
    @RiggsFolly The manual is *fantastic*, isn't it? – tadman Aug 31 '16 at 17:32
  • Add `mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);` to the top of your script. This will force any `mysqli_` errors to generate an Exception that you cannot miss or ignore. – RiggsFolly Aug 31 '16 at 17:34

2 Answers2

0

The problem is you are setting the $row variable directly even if it fails. This sets the $db_password to '' and the password should be ''. Hence it passes.

change the $row command to this

if ($row = mysqli_fetc_array($query) {

and add a } to the 3rd line from the bottom (the blank spot) and add your 'failure message' there as well

Forbs
  • 1,256
  • 1
  • 7
  • 9
0

You can use a simple query

$sql = "SELECT * FROM users WHERE username = '$username' and password= '$password'";
 $query = mysqli_query($db, $sql);
if(mysqli_num_rows($query) > 0)
{
  //Records matched process further
  $_SESSION['username'] = $username;
  $_SESSION['id'] = $id;
  header("Location: index.php");
  exit();
 } else {
   //Record not found throw error
   echo "Error: the information is not correct.";
 }

Hope this helps

Gandharv
  • 110
  • 6