Login and session system

[Anon]

I'm trying to write a include file for logging into a secure site using a cookie, storing the session in a DB for matching. It works pretty good, but since i want it to be absolutly secure i wonder if any of you have thing to add......

Thanks

################################################################################
##

Stig Woxholt ([e-mail])

09.12.200 ver. 1.0

##

Functions for handling the user login to the admin-site.

Has to be included to every file a administrator or customer is gonna access

cause we don't want any unauthorized personell in here.

##

DB - buildup for tables concerning the user-handling

##

users sessions

******** ********

userid id

customerid username

username timestamp

password session

flogon fromip

groups

##
################################################################################

// login function for handling the logon-page
// inputs are username and password filled in by the user

function user_login($username,$password)
{
global $REMOTE_ADDR,$cookieID;

$sql = "SELECT * FROM users WHERE username='$username' AND password='$password'";
$login_query = mysql_query($sql) or die(mysql_error());

$user = mysql_fetch_array($login_query);

if ($user != false)
{
$set = user_cookie();

if ($set == 1)
 {
  $uname = $user['username'];
  $session_sql = "INSERT INTO sessions (id,username,timestamp,session,fromip) VALUES ('$id','$uname',NOW(''),'$cookieID','$REMOTE_ADDR')";
  $user_sql    = "UPDATE users SET lastlogon=NOW('') WHERE username='$username'";

  $upduser_query    = mysql_query($user_sql) or die(mysql_error());
  $newsession_query = mysql_query($session_sql) or die(mysql_error());
  $id = mysql_insert_id();
 }
unset($username);
unset($password);


if ($user['groups'] == 'admin')
 { header("Location: aindex.php"); }
elseif ($user['groups'] == 'supplier')
 { header("Location: uindex.php"); }

}
else
{ header("Location: $adminroot/index.html"); }
}

// function for making the valid cookie

function user_cookie()
{
global $session,$cookieID;

if(!$session && !$cookieID)
{
$session = md5(uniqid(rand()));
SetCookie("cookieID", "$session", time() + 14400);
$set = 1;
}

return $set;
}

// Logs out the user by deleting the session from the database, an referring him
// to the login page

function user_logout($cookieID,$REMOTE_ADDR)
{
$sql = "SELECT * sessions WHERE session='$cookieID' AND fromip='$REMOTE_ADDR'";
$logout_query = mysql_query($sql) or die(mysql_error());
$user = mysql_fetch_array($sql);

if ($user != false)
{
$sql = "DELETE FROM sessions WHERE session='$cookieID' AND fromip='$REMOTE_ADDR'";
$logout_query = mysql_query($sql) or die(mysql_error());
header("Location: $adminroot/index.html");
}
else
{ header("Location: $adminroot/index.html"); }
}

// code for checking that the user is authorised to view the page

$verify_sql = "SELECT * FROM sessions WHERE session='$cookieID' AND fromip='$REMOTE_ADDR'";
$verify_query = mysql_query($verify_sql) or die(mysql_error());
$session = mysql_fetch_array($verify_query);

if ($session['session'] != $cookieID && $session['fromip'] != $REMOTE_ADDR)
{ header("Location: $adminroot/index.html"); }

[Anon]

Some remarks :

I don't understand why you use IP address in your WHERE clauses ?

• cookieID is enough to identify the user
• IP generally changes at each connection
• $REMOTE_ADDR is not user's IP if he uses a proxy

Another thing : if the user doesn't log out properly (I suppose there's a logout button somewhere), the session wil remain in the table. You should do some garbage-collecting somewhere to remove old sessions (from an administration page for instance).

Lastly, you forgot a FROM in your request "SELECT * sessions ...", but I suppose it's a mistake when typing in the message.