Login?

I have a mysql table (users) which holds all my login info.
When the username/password is filled out, it chec ks them using this bit of php


    $sql = "select * from users where username = '".$user."' && password = '".$pass."' limit 1";
     $result = mysql_query($sql); 
    $info = mysql_fetch_assoc($result);
    //Gives error if user dosen't exist
         if ($user != $info['username']) {
             header("location:login_fail.php?user");
        }
    //gives error if the password is wrong
         if (($pass != $info['password']) && ($user == $info['username'])) {
             header("location:login_fail.php?pass");
         } else {
            $id =  $info['id'];
              $_SESSION['logged'] = '1';
              $_SESSION['user'] = $user;
              $_SESSION['id'] = $id;
            header("location:login_success.php");
        }
mysql_close($db_connect); // Closes the connection.



But it seems like whtever I use, I get redirected to login_success.php even when I use a phony username or even a phony password with a correct username.
whats am I doing wrong?

Thx

After a header(“location:…”) redirect put an exit; to make sure that the rest of the code does not get executed.

There is a logical flaw in your code though: the query only returns a row if the user AND the password are correct. So your second IF will never be true. It’s impossible to have a correct user and a wrong password.

I do not really see the case for determining whether a username / password combination failed because of the username being wrong or the password being wrong - all you need to feed back is that the combination of the two was wrong – unless you want to track such failures yourself.

You do not really want to have a potential cracker discover “Ah, the username is correct - now all I need to do is crack the password”.

ie in pseudocode

if( query returns no results ){
header redirect
exit() // nod to guido2004
}else{
both user and pass were correct so log them in
}

ps You don’t show it, but do we presume $user and $pass have been escaped ready for use in this query?

else

Consider switching now to using Mysqli or PDO and investigate how to use prepared statements.

oh yes, I see what you mean cups

thanks, this worked.


    $user = mysql_prep($_POST['username']);
    $pass = mysql_prep($_POST['password']);
    $sql = "select * from users where username = '".$user."' && password = '".$pass."' limit 1";
     $result = mysql_query($sql); 
    $info = mysql_fetch_assoc($result);

    if(mysql_num_rows($result)==0){
             header("location:login_fail.php");
    //do this
    } else {
            $id =  $info['id'];
              $_SESSION['logged'] = '1';
              $_SESSION['user'] = $user;
              $_SESSION['id'] = $id;
            header("location:login_success.php");
    }

where mysql_prep is a php function to get the data ready to innsert into the database.


<?php
    function mysql_prep( $value ) {
        $magic_quotes_active = get_magic_quotes_gpc();
        $new_enough_php = function_exists( "mysql_real_escape_string" ); // i.e. PHP >= v4.3.0
        if( $new_enough_php ) { // PHP v4.3.0 or higher
            // undo any magic quote effects so mysql_real_escape_string can do the work
            if( $magic_quotes_active ) { $value = stripslashes( $value ); }
            $value = mysql_real_escape_string( $value );
        } else { // before PHP v4.3.0
            // if magic quotes aren't already on then add slashes manually
            if( !$magic_quotes_active ) { $value = addslashes( $value ); }
            // if magic quotes are active, then the slashes already exist
        }
        return $value;
    }
?>