0

I am trying to create a login form with PHP but it cannot login. I get incorrect password/username instead. I have triple checked the credentials in the database and they are correct. Please help

Here is the code

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Login</title>
<link rel="stylesheet" href="css/style.css" />
</head>
<body>
<?php
     require('db.php');
     session_start();
     // If form submitted, insert values into the database.
     if (isset($_POST['username'])){
         $username = $_POST['username'];
         $password = $_POST['password'];
         $username = stripslashes($username);
         $username = mysqli_real_escape_string($connection,$username);
         $password = stripslashes($password);
         $password = mysqli_real_escape_string($connection,$password);
         //Checking is user existing in the database or not
         $query = "SELECT * FROM `backuser` WHERE username='".$username."' and password='".md5($password)."'";
         $result = $connection->query($query) or die(mysqli_error());
         $rows = mysqli_num_rows($result);
         if($rows==0){
             $_SESSION['username'] = $username;
             header("Location: index.php"); // Redirect user to index.php
         }else{
             echo " <div class='form'><h3>Username/password is incorrect.</h3><br/>Click here to <a href='login.php'>Login</a></div>";
         }
     }else{
?>
<div class="form">
<h1>Log In</h1>
<form action="" method="post" name="login">
<input type="text" name="username" placeholder="Username" required />
<input type="password" name="password" placeholder="Password" required />
<input name="submit" type="submit" value="Login" />
</form>
</div>
<?php } ?>
</body>
</html>
Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
Vic
  • 108
  • 9
  • 2
    [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)*** Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! [Don't believe it?](http://stackoverflow.com/q/38297105/1011527) – Jay Blanchard Jul 25 '16 at 17:37
  • 2
    ***You really shouldn't use [MD5 password hashes](http://security.stackexchange.com/questions/19906/is-md5-considered-insecure)*** and you really should use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. Make sure you [don't escape passwords](http://stackoverflow.com/q/36628418/1011527) or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Jul 25 '16 at 17:37
  • Show us how you insert passwords into the database. – Jay Blanchard Jul 25 '16 at 17:39
  • 2
    Start basic debugging: echo out your generated query and run it manually. eliminate the pw check from the query and do that in your code. are you SURE you stored pws as md5 hashes in the db? otherwise you're doing `'hunter2' == md5('hunter2')` which will never match, returning zero rows. – Marc B Jul 25 '16 at 17:40
  • This seems to be beginner work. Considering the fact that he's using hashing and not storing plain text is a win. Cut the guy a break. – Nathan Robb Jul 25 '16 at 17:41
  • I agree with @MarcB. Make sure your query is outputting as expected. – Nathan Robb Jul 25 '16 at 17:42
  • 2
    What do you mean by "cut the guy a break"? These comments are fairly innocuous and are very helpful @NathanRobb The code here should not be used in production and could be considered dangerous. – Jay Blanchard Jul 25 '16 at 17:43
  • "You really shouldn't use MD5 password hashes and you really should use PHP's built-in functions to handle password security". The question is not about hashing security. – Nathan Robb Jul 25 '16 at 17:44
  • 2
    No, it's not @NathanRobb and that is why I posted it as a comment, not an answer ;-) – Jay Blanchard Jul 25 '16 at 17:45
  • 1
    @NathanRobb just because the statements don't relate directly to the question being asked, doesn't make the statements any less true – Martin Jul 25 '16 at 17:45
  • 3
    @NathanRobb: Also since he is just learning, it's probably better to teach him to do things the *right* way from the beginning. – gen_Eric Jul 25 '16 at 17:46
  • @VictorNgure. Any way we can see some of that debugging output? What's in the database, and what are you comparing it to? – Nathan Robb Jul 25 '16 at 17:46
  • @JayBlanchard i'm using a registration form to insert the data into the db. Would you like the code? – Vic Jul 25 '16 at 17:46
  • Yes, I would like the code. – Jay Blanchard Jul 25 '16 at 17:47
  • @NathanRobb the output is "Incorrect Username/Password". No errors just that. – Vic Jul 25 '16 at 17:48
  • 1
    If an answer solved your problem, consider accepting the answer. Here's how http://meta.stackexchange.com/questions/5234/how-does-accepting-an-answer-work then return here and do the same with the tick/checkmark till it turns green. This informs the community, a solution was found. Otherwise, others may think the question is still open and may want to post (more) answers. *Welcome to Stack!* – Jay Blanchard Jul 25 '16 at 17:58
  • @JayBlanchard i'll do that. Thank you for the tip – Vic Jul 25 '16 at 18:03
  • @MarcB i cannot understand what you've written. I'm rather green to php so could you kindly simplify what you've just said. – Vic Jul 25 '16 at 18:15
  • PHP code should be above HTML code, especially when calling session_start(). – Nitin Jul 25 '16 at 18:26

3 Answers3

4

You have an error in your logic. Change if($rows==0) to if($rows!=0). If the user is in DB, you should have at least 1 record returned and mysqli_num_rows() should return a number >= 1.

Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
sarasvati
  • 792
  • 12
  • 30
  • @JayBlanchard unfortunately I don't have OP's source code and DB dump. The only way of helping him (in my point of view) is stating the obvious logical errors in the code. *Ad explanation:* I explained the reason immediately after suggesting the fix. – sarasvati Jul 25 '16 at 17:53
  • @sarasvati Tried that and unfortunately nothing's changed. – Vic Jul 25 '16 at 18:01
1

Sarasvati's answer appears to fix your immediate issue, however I would like to also suggest:

  • Use PHP Password hashing built in functions (PHP 5.5+ and 5.3) for hashing passwords. MD5 is not cyptographically secure and is not advised for password storage or checking.

  • If you are new to PHP it is very very much worth your time exploring using Prepared Statements with your code. This seperates code from user input and so greatly reduces the risk of SQL injection and Database Compromise.

  • When you have a header which redirects you should be immediately following this header with an exit statement, as the rest of the PHP code will still be run before the header is followed.

        $_SESSION['username'] = $username;
         header("Location: index.php"); // Redirect user to index.php
         exit;
    
  • Do not use text editing functions such as stripslashes() on password fields, if someone has a slash in their password, it will be stripped and they will not be able to log in. Passwords should be any character, not just a-z or 0-9 etc.

  • For MySQLi_ [procedural] error reporting you need to provide the connection details:

    die(mysqli_error($connection));
    

    otherwise the report will not give you anything.

  • PHP session_start() should be before anything is output to the browser, so this needs to be at the very top of your PHP page. (Thanks to Nitin for this, I missed this originally) .

Community
  • 1
  • 1
Martin
  • 22,212
  • 11
  • 70
  • 132
0

The problem is that i had not allocated enough storage(character length) in my database to store the hash value. I adjusted the length value to 30 and it works like a charm.

Vic
  • 108
  • 9