1

I have a PHP script using PDO where I check a user's submitted email/password against a database. If the query returns a row, it is supposed to take the user to a success page, or if the credentials are incorrect they are supposed to be taken to a failed login page. However, the user is always taken to the fail page.

$sql = "SELECT email, password FROM user WHERE email= $email AND password = $password";
$stm = $db->prepare($sql);
$stm->execute();

$result = $stm->fetchColumn();

if ($result !== FALSE) {

     header('Location: ./success.html');
}
else {

    header('Location: ./failed.html');

}
miken32
  • 42,008
  • 16
  • 111
  • 154
Jeremy
  • 59
  • 6
  • "I feel like the comparison I'm using in my if to determine which page the user gets sent to is incorrect." Why? Does it work? – miken32 Oct 05 '18 at 03:30
  • It will take me to one page but not the other, regardless of whether the user input's the correct email/password or not. – Jeremy Oct 05 '18 at 18:07
  • Do you feel like sharing which page it takes you to? – miken32 Oct 05 '18 at 18:10
  • 1
    Sorry, the failed paged. – Jeremy Oct 05 '18 at 18:21
  • I may not be understanding the logic of how it works. If I'm not mistaken, the fetchColumn() function returns an array with the row(s) from the query. Therefore, if fetchColumn returns an array, the query execute successfully, and if there's no array then the user entered credentials wrong (assuming everything else is working properly). Then the IF statement checks whether an array was returned or if a value of false (for no array) was returns and makes a decision. – Jeremy Oct 05 '18 at 18:25
  • 1
    Ok first thing you need to do is some error checking. fetchColumn is returning false or something that evaluates to false. If you check the results of prepare and execute before using them, you should be able to see your problem. – miken32 Oct 05 '18 at 18:26
  • Thanks! I did a vardump($result) using correct credentials and it showed false, so at least my IF logic was correct. Now to figure out why it's returning false. – Jeremy Oct 05 '18 at 18:30
  • Realized I didn't bind params with the prepare(). Thanks. – Jeremy Oct 05 '18 at 18:44

1 Answers1

1

Your original problem was simply missing quotes around the variables inserted into the query.

Just fixing that problem would leave you vulnerable to SQL injection attacks. Making proper use of statement preparation and execution will solve that problem. And never, never, store plaintext passwords. When you save them, use password_hash and then use code like this to verify them.

$password = $_POST["password"];
$email = $_POST["email"];
$sql = "SELECT password FROM user WHERE email= ?";
$stm = $db->prepare($sql);
$stm->execute([$email]);

$result = $stm->fetchColumn();    
if ($result !== FALSE) {
    if (password_verify($password, $result[0])) {
         header("Location: ./success.html);
        exit;
    }
}
header("Location: ./failed.html");

More details on password hashing can be found elsewhere on SO.

And please note that for brevity I'm not checking the result of the prepare() or execute() functions. You should be doing this.

miken32
  • 42,008
  • 16
  • 111
  • 154
  • Thanks, I realized I didn't bind params with the prepare() before I saw your post. It's working as intended now. Thanks for touching on the password hashing, I was getting to that but one wanted the logic to work correctly before adding complexity. – Jeremy Oct 05 '18 at 18:46