-1

i have a login form that transmit the information to the php code in the same page but even the username and password are correct it always ignore him this is my php code:

$mail = test_input($_POST["email"]);
//check if the email address format is valid
  if (!filter_var($mail, FILTER_VALIDATE_EMAIL)) {
  echo "<h3>Invalid email format</h3>";
  }

$pass = trim($_POST["password"]);
$stmtn="SELECT first_name, email , password FROM `users` WHERE email= '$mail'";

$result = mysqli_query($connection,$stmtn);
$rows=  mysqli_fetch_assoc($result);

if($rows && $rows['password']==  md5($pass)){
    session_start();
    $_SESSION['username']=$rows['first_name'];
    header("Location: index.php");
}else{
    echo "<h3> Sorry! your email or password is incorrect </h3>";
    echo "<h3> Please <strong><a href='login.php'>login</a></strong> again to your account </h3>";

    echo var_dump($rows);


}



the statements in the else part always are executed and the result of the var_dump for the array $rows is equal to the information in the db exactly

and this is the code of the function test_input

<?php
function test_input($data) {
  $data = trim($data);
  $data = stripslashes($data);
  $data = htmlspecialchars($data);
  return $data;
}
  • Where is "$pass" defined? – Iofacture Oct 25 '16 at 20:05
  • yes, of course! and the test_input is a function to validate inputs it is already defined – A.alhamdani Oct 25 '16 at 20:07
  • are the hash values for $rows['password'] and md5($pass) the same? Have you tried doing a var dump or a print to see if there is an issue there? – Iofacture Oct 25 '16 at 20:08
  • that was my query when i remove the hash and insert a user without hashed password then login it works – A.alhamdani Oct 25 '16 at 20:11
  • var_dump print the same – A.alhamdani Oct 25 '16 at 20:12
  • 2
    You vulnerable to [sql injection attacks](http://bobby-tables.com) – Marc B Oct 25 '16 at 20:13
  • $rows && $rows['password']== md5($pass) if rows isnt empty or null and rows password and md5 pass result is the same you should be getting into that block of code – Iofacture Oct 25 '16 at 20:14
  • i know that @Matt1776 but it is not happen – A.alhamdani Oct 25 '16 at 20:20
  • My only suggestion then is to try and make sure both $rows['password'] and $pass have been trimmed for extra whitespace – Iofacture Oct 25 '16 at 20:22
  • for example i insert a user to the database and this what var_dump prints array (size=3) 'first_name' => string 'osama' (length=5) 'email' => string 'osama1234@gmail.com' (length=19) 'password' => string '81dc9bdb52d04dc20036' (length=20) and it is the same – A.alhamdani Oct 25 '16 at 20:22
  • what is test_input() doing to the password? – useyourillusiontoo Oct 25 '16 at 20:22
  • @useyourillusiontoo i add the code to my question – A.alhamdani Oct 25 '16 at 20:24
  • does the password you are testing contain any of the following: " & ' > < If so htmlspecialchars will be converting them to the html entity. Likewise any slashes will be removed by stripslashes – useyourillusiontoo Oct 25 '16 at 20:36
  • 1
    I hope ure not using test_input on d password. If the user's password contains special characters n what not, it'ld b stripped making it not the same as what the user entered orginally and will possibly not checkout. You might want to looking into dat. Since, ure encoding with md5, why test? – Mueyiwa Moses Ikomi Oct 25 '16 at 20:37
  • @Matt1776 i used the var_dump to print the value of md5($pass) i discover that it is not the same sorry ,but why md5 gives another value it adds some string to the actual string and how can i fix it? – A.alhamdani Oct 25 '16 at 20:37
  • get rid of the test_input() function and just use trim() does that make it work? – useyourillusiontoo Oct 25 '16 at 20:39
  • Please post the code that inserts the user. Also please show the SQL table definition for the user table. – Iofacture Oct 25 '16 at 20:47
  • 1
    ***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 Oct 25 '16 at 21:36

1 Answers1

2
  • Please stop using stripslashes, it's really not needed. Instead you should properly clean your user input using a filter_var or a regex.

  • After you've set the header() to redirect you should always and immediately set an exit statement to cease the script execution.

  • You really, REALLY should use parameterised queries with MySQLi to stop your SQL being abused and attacked (take a peak at this question just for a flavour of the layout of parameterised MySQLi)

  • Stop using md5. Now. if your PHP is above 5.3 you really, really should be using password_hash and password_verify for your and your users security. Good habits start at home.

    • Also don't trim user passwords. Don't make any changes to a password variable at all (until after they've been password_verifyed), because if I want to have " dickens !" as my password, why the hell shouldn't I?
  • With the above code changes in place there is no need for the test_input function at all on your script.

    • And there is no need to test_input on the email address because you validate it straight afterwards with filter_var. Inefficient.

Good luck.

Community
  • 1
  • 1
Martin
  • 22,212
  • 11
  • 70
  • 132
  • There was so much wrong I didn't know how to start, but you've done a nice job here :) I was trying to fix the problem with his current code but its difficult given the amount of information. – Iofacture Oct 25 '16 at 20:56
  • @Matt1776 I love bullet points. Although I can't give a specific answer to OPs specific issue, if they implement the changes outlined in my answer they fit best practise and I am willing to bet their issue will disappear `:-)` – Martin Oct 25 '16 at 20:58
  • thank you @Martin what i missed is the exit after the header. and with the password_hash () – A.alhamdani Oct 27 '16 at 14:50