Auto-login with cookies

Question

Hi,

I've been trying to make an Auto-login feature to my login script for 2 nights now.

So right down to the problem.

I have a field in my users table called session_key varchar(255) in which i store the users IP hashed with md5.

If the cookie is set, session vars will be set and if not a form will appear.

When they login and if they have checked a checkbox named autologin it will store their md5 hashed ip in field session_key.

Now if they close their browser and come back, I check that their cookie is equal to session_key in the database.

This is the messy code! I didnt think it would be this hard to implent a auto-login function..

session_start();

// see if the auto-login cookie exists, if so set sessions vars
if (isset($_COOKIE['autologin'])) {
$user=mysql_query("select * from users where session_key = $_COOKIE[autologin]");
$row3=mysql_fetch_assoc($user);



        if ($_COOKIE['autologin'] == $row3['session_key']) {

           $_SESSION['id'] = $row3['id'];
     header("Location: login.php");
        }
    }


// User pressed "Login"
if (count($_POST)) {

$result = mysql_query("SELECT id FROM users 
                       WHERE username = '".mysql_real_escape_string($_POST['username'])."' 
                       AND password = '".mysql_real_escape_string($_POST['password'])."' ");


if (mysql_num_rows($result) == 0) {
     $error = '<script>alert("Wrong username and/or password.\nTry again.")    </script>';
} else {
    $_SESSION['id'] = mysql_result($result, 0, 'id');
 header("Location: login.php");
 mysql_query("UPDATE users SET session_key = '".md5($_SERVER['REMOTE_ADDR'])."'");
 setcookie("autologin", md5($_SERVER['REMOTE_ADDR']), time()+3600);
}

}



if (isset($_SESSION['id'])) {

exit('You are logged in!');

}



<form method="post">

<tr>
<td valign="top" width="95px"><b>Username:</b></td>
<td><input type="text" name="username" size="22" /></td>
</tr>

<tr>
<td valign="top"><b>Password:</b></td>
<td><input type="password" name="password" size="22" /></td>
</tr>

<tr>
<td valign="top"><b>Auto-login:</b></td>
<td><input type="checkbox" name="autologin" /></td>
</tr>

<tr>
<td>&nbsp;</td>
<td><input type="submit" value="Login" /></td>
</tr>

</form>

Is this even safe? I mean you can easily manipulate cookies. But I mean youtube has this function! How the hell to they do it?

I just cant get it to work!! Im going crazy. Can someone give me some help? Before I go on a rampage!

Answer 1

First of all:

    // User pressed "Login"
if (count($_POST)) {

$result = mysql_query("SELECT id FROM users 
                       WHERE username = '".mysql_real_escape_string($_POST['username'])."' 
                       AND password = '".mysql_real_escape_string($_POST['password'])."' ");


if (mysql_num_rows($result) == 0) {
            $error = '<script>alert("Wrong username and/or password.\nTry again.")    </script>';
} else {
    $_SESSION['id'] = mysql_result($result, 0, 'id');
        header("Location: login.php");
        mysql_query("UPDATE users SET session_key = '".md5($_SERVER['REMOTE_ADDR'])."'");
        setcookie("autologin", md5($_SERVER['REMOTE_ADDR']), time()+3600);
}

}

In the last section where you have written:

header("Location: login.php");

You are updating database and setting cookie, after you are redirecting the user by header function. So your code never updates and set that cookie. Change it to:

 // User pressed "Login"
if (count($_POST)) {

$result = mysql_query("SELECT id FROM users 
                       WHERE username = '".mysql_real_escape_string($_POST['username'])."' 
                       AND password = '".mysql_real_escape_string($_POST['password'])."' ");


if (mysql_num_rows($result) == 0) {
            $error = '<script>alert("Wrong username and/or password.\nTry again.")    </script>';
} else {
    $_SESSION['id'] = mysql_result($result, 0, 'id');
        mysql_query("UPDATE users SET session_key = '".md5($_SERVER['REMOTE_ADDR'])."'");
        setcookie("autologin", md5($_SERVER['REMOTE_ADDR']), time()+3600);
        header("Location: login.php");

}

}

Answer 2

First, don't ever stick strings from your user (this includes cookies) into your SQL without escaping it first.

Secondly, session keys need to be unique, and hard to guess. It looks like all I have to do to steal someone's session is to know their IP address. Worse yet, I can steal their session by accident by having the same IP as them (which is very often the case if I'm in the same building they are).

Third, your indentation doesn't match your curly brackets, so it's hard to figure out what's going on.

Fourth, this "if ($_COOKIE['autologin'] == $row3['session_key'])" is redundant, since you already checked in SQL that they match.

Fifth, on your 5th line, you don't have quote marks around your (unescaped) value, so if your code execution had ever gotten to this point, you should get an SQL syntax error.

I suggest you make much much simpler smaller pieces of code that you test one at a time. For example, start with setting a cookie, and testing that the value comes through. Then only after you get that working, add some code to check if a database record matches your cookie. And make sure to encode everything you put in sql, so if someone sets their cookie to "; drop database xxx" or whatever, you aren't screwed.