php mysql where statement with AND return true even if the second condition is false

Question

I am running a mysql statement which actually evaluates to true despite the incorrectness of a value. Bellow is the function

<?php
public function login_user($username, $password){

      if(!empty($username) && !empty($password)){
        $sql = "SELECT * FROM `users` WHERE `user_name`='$username' AND `user_password`='$password'";
        $query = $this->link->query($sql);

        if($this->link->error){
            //return false;
            $this->log_db_error($this->link->error, $sql);
            return false;
        }
        else{

            return true;
        }
    }
    else{
      return false;
    }
  }
?>

Calling this function with the params and passing a correct username and a wrong password actually returns true, where am I messing up? Any help

First fix your SQL injection https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php — Raymond Nijland, Feb 18 '18 at 13:06
Why are you expecting an error? The SELECT query will just return 0 results if no rows match the conditions, which is not an error. — Raul Sauco, Feb 18 '18 at 13:07
Your query seems to be valid. So, you never returns false. But you have to fetch the result or check the number of rows returned by the query to know if there is a match in the database. — Syscall, Feb 18 '18 at 13:07
yeah I have already a function that deals with that @RaymondNijland — BekiTheMe, Feb 18 '18 at 13:08
If you mean you are escaping inputs then Your script is STILL wide open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) 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, Feb 18 '18 at 13:08
It also looks like you are storing PLAIN TEXT PASSWORDS. Look at `password_hash()` and `password_verify()` — RiggsFolly, Feb 18 '18 at 13:10
@RiggsFolly, sure, let me work on it Immediately. Thanks guys ALOT! — BekiTheMe, Feb 18 '18 at 13:11
This is not the best way to check if the password is valid, it means you are saving the passwords in the database in plain text, a bad practice. You should be encrypting the passwords before you save them, then have your SQL find the user and use the same function to test if the entered password matches, if it does return `true` in any other case return `false` — Raul Sauco, Feb 18 '18 at 13:12
@RaulSauco, passwords in the db are encrypted, this was just a demonstration so I could understand where I was doing wrong. — BekiTheMe, Feb 18 '18 at 13:15

score 0 · Accepted Answer · answered Feb 18 '18 at 13:14

Because the sql query is totally valid even if the combination of username and password doesn't exist in database.

You can achieve what you want by: Checking if query returns empty dataset.

Some Tips: 1. Your query is prone to sql injection. It is good practice to use prepare and then bind all the input parameters. 2. It's good to keep passwords in hash (Like md5 or BCrypt) in database.

php mysql where statement with AND return true even if the second condition is false

1 Answers1