Login code algorithm help needed

[jackz15]

Hi everyone, could someone take a look at my convoluted login algorithm? It works fine except if I close out from the site and don't return for a while and open the page:
1. cookie logs me in, sets phpsseid
2. by refreshing the page, i get logged out with login cookie deleted

when i first login manually, no matter how i refresh, i don't get logged out. Im not really sure why this is. Could someone help take a look at my code?

  //auto login with cookie
if(isset($_COOKIE['login']) && !(isset($_SESSION['uid']))){
$coo=$_COOKIE['login'];
$cookie_array = unserialize(base64_decode($coo));
$uid=$cookie_array[0];
$cookie_data=$cookie_array[1];

if (!$uid or !$cookie_data) return;
$q = "SELECT * FROM Users WHERE uid='$uid'and cookie='$cookie_data'";
$ro=mysqli_query($dbc,$q) or trigger_error("Query: $qn<br />MySQL Error: " . mysqli_error($dbc));
if (@mysqli_num_rows($ro) == 1) { // A match was made.

			// Register the values & redirect:
	$_SESSION = mysqli_fetch_array ($ro, MYSQLI_ASSOC);
			mysqli_free_result($ro);
if($_SESSION['preferences']==NULL){
                      $_SESSION['pref']=1;
                        }else{
                     $_SESSION['pref']=0;
                }
        }else{
        setcookie ("login", "", time() - 3600);
        unset($_COOKIE['login']);
        header("Location:login.php");
        }

}else if(isset($_SESSION['uid']) && isset($_COOKIE['login'])){

$u=$_SESSION['uid'];
$ip=$_SERVER['REMOTE_ADDR'];
$coo=$_COOKIE['login'];
$cookie_array = unserialize(base64_decode($coo));
$uid=$cookie_array[0];
$cookie_data=$cookie_array[1];

$sql = "SELECT * FROM Users WHERE uid='$u' and ip='$ip' and cookie='$cookie_data'";
$r=mysqli_query($dbc,$sql) or trigger_error("Query: $sqln<br />MySQL Error: " . mysqli_error($dbc));

if (@mysqli_num_rows($r) == 1) {

}else{

session_destroy();
setcookie ("login", "", time() - 3600);
unset($_COOKIE['login']);
header("Location:login.php");

}
}else if(!(isset($_SESSION['uid']))){
       header("Location:login.php");
}

basically there are two thing involved, the login cookie which remember login and the session id that identifies you as being logged in. Hope the code isn't too hard to read ...

 

[jpadie]

as per the manual i suggest you change this code block as shown

[red]$_SESSION=array();
if (ini_get("session.use_cookies")) {
    $params = session_get_cookie_params();
    setcookie(session_name(), '', time() - 42000,
        $params["path"], $params["domain"],
        $params["secure"], $params["httponly"]
    );
}
[/red]
session_destroy();
setcookie ("login", "", time() - 3600);
unset($_COOKIE['login']);
header("Location:login.php");

 

[jackz15]

is that all I have to do? Is there anything else wrong with my code?

Thanks for the help!

 

[jpadie]

the above code should fix the session destruction properly.

are you trying to provide a remember me function whilst also protecting against man-in-the-middle / session hijack attacks?

i note that you are not testing for when there IS a uid in the session and IS NOT a login cookie set. this would occur when the session is not properly destroyed and yet the login cookie is properly destroyed. the absence of this test means that the conditionals never fire so code execution continues after this code block.

some other thoughts:

what do you expect return to do? this only works in a function/method.
it is better to do a count(*) than retrieve a recordset just to determine the number of rows. however I can see the point here.
i would use array_merge($_SESSION, $row) rather than a direct assignment. that way any other session data you store gets kept.
for double protection in your setcookie lines set the value to FALSE when you want to delete the cookie.