-3

I'm building an app when i'm able to hash user passwords by using MD5. For the insertion it works correctly, but when i'm authenticating, it tells me the password is wrong

require_once('connect.php');
$data = json_decode(file_get_contents("php://input"));
$email = ($data->email);
$password = ($data->password);

$row = $conn->query("
    SELECT * 
    FROM user 
    WHERE email='".$email."' 
        AND password='".md5.$password."'
");
$row->setFetchMode(PDO::FETCH_ASSOC);

$userdetails = $row->fetchAll();
$user = $row->rowCount();

$error_message=array("message"=>("wrong"));

if ($user == 0) {
    echo json_encode($error_message);
} else {
    session_start();
    $_SESSION['user']=$userdetails;
    echo json_encode($userdetails);
}
Barry
  • 3,303
  • 7
  • 23
  • 42
user6579134
  • 749
  • 3
  • 10
  • 35
  • 3
    md5 is not encryption. It is hashing. And you shouldn't be using md5 anymore because it is considered insecure. See https://www.owasp.org/index.php/Cryptographic_Storage_Cheat_Sheet and http://php.net/manual/en/book.password.php – Gordon Oct 15 '18 at 11:53
  • 3
    Your error is here `password='".md5.$password.` – Masivuye Cokile Oct 15 '18 at 11:54
  • thanks for the correction, just edited the question – user6579134 Oct 15 '18 at 11:54
  • is md5($password) – Roy Bogado Oct 15 '18 at 11:55
  • Please use a prepared statement with bind parameters. You are already using PDO. Also check https://secure.php.net/manual/en/function.password-hash.php and https://secure.php.net/manual/en/function.password-verify.php for how to use proper password handling in php – rypskar Oct 15 '18 at 12:00
  • Like Gordon has said, MD5 is not a secure way to store passwords, not even with a salt. Have a look at PHP's [password_hash](http://php.net/manual/en/function.password-hash.php) and [password_verify](http://php.net/manual/en/function.password-verify.php) functions. They are very easy to use. This is also a useful question about [why not to use md5 for password hashing](https://stackoverflow.com/questions/30496061/why-not-use-md5-for-password-hashing) – frobinsonj Oct 15 '18 at 12:01
  • BTW, your code should give you an error of undefined constant md5 – Masivuye Cokile Oct 15 '18 at 12:07
  • @MasivuyeCokile why an undefined constant? the md5 and input password are concatenated and should produce `md5InputtedPassword` since both are strings. – Funk Forty Niner Oct 15 '18 at 12:10
  • @FunkFortyNiner just tested this it gives **Warning**: Use of undefined constant md5 - assumed 'md5' (this will throw an Error in a future version of PHP) in with php 7.2.4 – Masivuye Cokile Oct 15 '18 at 12:14
  • @MasivuyeCokile *Hm...*, odd. Oh well, if you tested it ;-) – Funk Forty Niner Oct 15 '18 at 12:15
  • 1
    the `.md5.$password.` is not enclosed with any quotation marks to make it a string if it was `".md5.$password."` then it was gonna be a string the entire statement @FunkFortyNiner – Masivuye Cokile Oct 15 '18 at 12:18

3 Answers3

5

Change

$row = $conn->query("SELECT * from user where email='".$email."' and password='".md5.$password."'");

to

$row = $conn->query("SELECT * from user where email='".$email."' and password='".md5($password)."'");

NB : You should use prepared statements and also don't use md5() use password_hash() and password_verify()

Then in your registration page you will have.

$hash = password_hash($password,password_default);  // store this hash

Then your login page.

$stmt = $conn->query("SELECT * from user where email= ? ");
$stmt->execute(array($email));

$row = $stmt->fetch();

if(password_verify($password,$row['passwordFromDB'])){

    session_start();
    $_SESSION['user']=$userdetails;
    //return what needs to be returned
}else{
    $error_message=array("message"=>("wrong"));
    echo json_encode($error_message);
}

NB : Make sure your database column have a length of 60+

Masivuye Cokile
  • 4,754
  • 3
  • 19
  • 34
1

Your code is not secure for different reasons:

Use prepared statements with your pdo instance.

Instead of using pdo's query function, use prepare PDO::prepare, this will flatter the SQL injection risk (it would be still possible though).

Do not use md5 as your hashing algorithm

MD5 is not secure anymore, use the standard password_hash/password_verify functions that PHP offers. password_hash()

By the way, your error is in the query

$row = $conn->query("SELECT * from user where email='".$email."'
and password='".md5.$password."'");

it is md5($password) not md5.$password

Alpe89
  • 299
  • 2
  • 11
0

I think you meant:

$conn->query("SELECT * from user where email='".$email."' and password='".md5($password)."'");

typing md5.$password will probably evaluate to the string md5 followed by the input password. It might also throw an warning, depending on your PHP settings.

Ben Hillier
  • 2,126
  • 1
  • 10
  • 15