1

I wrote a function in PHP to check username and password from MySQL database and store as session after successful validation. But whatever value I enter to the input box it approves it and made a successful login. My Login code doesn't work

here it is

function queryByUserAndPass($tableName, $username, $password){
    $queryStatement = "SELECT * FROM ".$tableName." WHERE username='".$username."' 
                      AND password='".$password."' LIMIT 1";
    return $queryStatement;
}

function checkLogIn() {

    if(isset($_POST['submit'])){
        $username = $_POST['username'];
        $password = $_POST['password'];
        $queryState = queryByUserAndPass("nepal_users", $username, $password);
        if( $resultQuery = mysql_query($queryState) ){
            $found_user= mysql_fetch_array($resultQuery);
            $_SESSION['id']=$found_user['id'];
            $_SESSION['username']=$found_user['username'];
            $message="succesful log in ".$_SESSION['username'];
            header("location:home.php");
            exit;
        }else {
            $message="error in log in";
        }    

    }

}

Please tell me what is wrong in this code and why it is not working.

hakre
  • 193,403
  • 52
  • 435
  • 836
monk
  • 4,727
  • 13
  • 45
  • 67
  • 1
    function queryByUserAndPass($tableName, $username, $password){ $queryStatement = "SELECT * FROM ".$tableName." WHERE username='".$username."' AND password='".$password."' LIMIT 1"; return $queryStatement; } – monk Feb 16 '12 at 18:32
  • 1
    Dont you have some encryption on the users password ?? If you have , You have to apply the same method – mlinuxgada Feb 16 '12 at 18:34
  • have you initialized your session with session_start(); – David Richard Feb 16 '12 at 18:35
  • 3
    Remember to use `mysql_real_escape_string` on your username and password input before putting them in the database - otherwise anyone could put `' or 1='1` as the password and gain access to the username of their choice. – Niet the Dark Absol Feb 16 '12 at 18:36
  • 1
    FYI: [SQL injection](http://stackoverflow.com/a/332367/29995), [password encryption](http://stackoverflow.com/a/3107832/29995) – Rob Hruska Feb 16 '12 at 18:39
  • 1
    Pls sanitise your data flow/prepared statements, mysql_escape_string, mysql_real_escape_string, addslashes .../ and use always ecryption hash algorithms to protect your passwords/plus salts/. – mlinuxgada Feb 16 '12 at 18:40

2 Answers2

3

mysql_query always returns a resource on success, or false if there is an error in your code.

What you want to do is:

$resultQuery = mysql_query($queryState);
if( $found_user = mysql_fetch_assoc($resultQuery)) {
    // do login stuff
    // note I used "fetch_assoc" above, because you don't use numeric indices here.
}
else $message = "Error";
Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
0

you dont check if username and password are correct!

    if( $resultQuery = mysql_query($queryState) ){
        $found_user = mysql_fetch_array($resultQuery);
        if ($username === $found_user['username'] &&
          $password === $found_user['password']) {
            $_SESSION['id']=$found_user['id'];
            $_SESSION['username']=$found_user['username'];
            $message="succesful log in ".$_SESSION['username'];
            header("Location: home.php");
            exit;
        } else {
            echo "wrong username or password";
        }
    }else {
        $message="error from database: " . mysql_errno($resultQuery);
    }

IMPORTANT NOTE

The code above just illustrates where the logic problem in your code is. There are two other serious issues in it:

  1. Its vulnerable to SQL injection. Using prepared statements is common practise. If prepared statemements are not availabe to you, use something like $SAFE_USER_DATA = array_map('mysql_real_escape_string', $_POST); and use this instead of directly reading $_POSTdata. That way you ensure no malicious user can modify your SQL statements to gain access to your system.

  2. You should never store passwords as plain text - storing a salted SHA1 password hash is encouraged. do NOT simply use md5($password). Attackers can easily decrypt those with the help of rainbow tables

Kaii
  • 20,122
  • 3
  • 38
  • 60
  • 2
    It is safe to assume that the username and password are put into the query by the `queryByUserAndPass` function. – Niet the Dark Absol Feb 16 '12 at 18:35
  • @Kolink you are right, the OP posted the function in a comment after i posted my answer. However, the quick fix illustrated here should work, too. despite the serious issues regarding SQL injection and missing password hashes, though.. – Kaii Feb 16 '12 at 22:57