Secure Login script?

pablodelapena

recently I've been using thiss cript to authenticate my users. Although suddenly I've had doubts as to its security. what do you guys think?

Note: passwords are encrypted in the database using the password() function in MySQL.


<?php
REQUIRE_ONCE('config.php');

CheckIP();

//If any $MCuserID variable is set then we go about destroying the session.
if (isset($_SESSION['MCuserID'])) {
session_destroy();
}


//First we generate a random session ID
$sessionID = rand(0, 100000000);

//Now we store the users IP in a varable
$sessionIP = $_SERVER['REMOTE_ADDR'];

//Special charaterise the username
$_POST['LoginID'] = htmlspecialchars($_POST['LoginID'], ENT_QUOTES);


//Now we check to see if the user is allowed to login.
$query = "SELECT * FROM userinfo WHERE username='".$_POST['LoginID']."'";
$result = mysql_query($query);
CheckMySQL();

$affected = mysql_num_rows($result);
if ($affected != 1) {
   $BadLogin="invalid";
   BadLogin();
header ("Location: logon.php?error=invalid&NoCount=1&errorcount=1");
exit();
}

$row = mysql_fetch_array($result);

//Now we check the user's status. If no user is found then they will have already been redirected.
if ($row['status'] == "pending") {
   $BadLogin="pending";
   BadLogin();
header ("Location: logon.php?error=pending&NoCount=1&errorcount=2");
exit();
} elseif ($row['status'] == "banned") {
   $BadLogin="banned";
   BadLogin();
header ("Location: logon.php?error=banned&NoCount=1&errorcount=3");
exit();
}

//If the user is a donator we have to tell the sessions table that.
if ($row['donatorvalue'] == "nondonator") {
$donator = "no";
} else {
$donator = "yes";
}

//Assuming that the user is active we then authenticate them

//Check username and password
$query = "SELECT * FROM userinfo WHERE username = '".$_POST['LoginID']."'
 AND password = password('".$_POST['password']."')";
$result = mysql_query($query);

$valid_login = mysql_num_rows($result);
if ($valid_login != "1") {
   $BadLogin="invalid";
   BadLogin();
header ("location: logon.php?error=invalid&NoCount=1&errorcount=4");
exit();
} else {


//The user may already be in the sessions table. Lets check it out.
$query = "SELECT * FROM sessions WHERE MCsessionID='".$_POST['LoginID']."'";
$result = mysql_query($query);
CheckMySQL();

$loggedin = mysql_num_rows($result);

if ($loggedin == 1) {

//If this returns a variable we delete the user
//Add to array first
$row = mysql_fetch_array($result);

$query = "DELETE FROM sessions WHERE sessionID='".$row['sessionID']."'";
$result = mysql_query($query);
CheckMySQL();


}


$timenow = time();

//Does the user have a profile? we need to find that out now!
$query = "SELECT * FROM userinfo WHERE username='".$_POST['LoginID']."'";
$result = mysql_query($query);
CheckMySQL();

//Add info to an array
$row = mysql_fetch_array($result);
$profile = $row['profile_exist'];


//We INSERT the users information into the sessions table.
$query_guest = "INSERT INTO sessions (sessionID, MCsessionID, sessionIP, time,  donator, profile)
 VALUES ('$sessionID', '".$_POST['LoginID']."', '$sessionIP', '$timenow', '$donator', '$profile')";
$result_guest = mysql_query($query_guest);
CheckMySQL();

//update the users last IP
$query = "UPDATE userinfo SET LastIP='$sessionIP' WHERE username='".$_POST['LoginID']."'";
$result = mysql_query($query);
CheckMySQL();


//We set the session variables for the user

session_start();
$_SESSION['MCuserID'] = $_POST['LoginID'];
$_SESSION['sessionID'] = $sessionID;

//Set the Last_Login time and online_flag, start timers also
//Reset pages_viewed
$last_login = gmdate("YmdHis");
$last_login_view = gmdate("d-m-Y H:i");
$query = "UPDATE userinfo SET last_login='$last_login',online_flag='online', login_time='$timenow', 
last_login_view='$last_login_view', pages_viewed='0' WHERE username='".$_SESSION['MCuserID']."'";
$result=mysql_query($query);
CheckMySQL();

//Set cookie for the login page.
setcookie ("LoginIDcookie", $_POST['LoginID'],mktime (0, 1, 1, 2, 2100));



//Now we redirect to the homepage.
header ("Location: index.php");
exit();



}

?>

planetsim

password() isnt all that great compared to PHP's md5();

password() algorithm is similar but not nearly as good as md5(). The only way a password can be gotten is by brute force or security holes else where in the script.

$query = "SELECT * FROM userinfo WHERE username='".$_POST['LoginID']."'";

You must use addslashes to stop users from doing this in the form

Username: (form ->) ' OR 1=1 that would get a user in very easily. Besides that nothing else stands out

pablodelapena

Ok, Ive heard from variious sources that md5 is better, but I think it's too late now.

I have around 300 users who ahve all encrypted their password using password(). Is there anyway I could change this to md5?

Also would simply adding this line in be sufficient:

$_POST['LoginID'] = addslashes($_POST['LoginID']);

Will that not confuse the database? Also even though I'm using htmlspecialchars() would is till need to do this?

Thanks for your response though 🙂

planetsim

Why not just doing it in the query. Or like this

foreach ($_POST as $key => $value) {
  $_POST[$key] = addslashes($value);
}

That could be done better maybe but at least you wont need to do it in every query also make sure you use that for $_GET as well. Id put both loops into a global file that gets included in every file.

As for password() its never too late to change just implement it. When a user trys to login if fail they can always use a Lost Password of course they must have Email which is recorded in the database. Then just validate it create a random password, once that is done send them the email (do this and not in the browser because any user can guess the email+username and it would be useless).

Then once they have got the new password they must be able to change it to something they can remember so they must have the option of chaning the password.

Im sure this isnt too hard to implement at all, and it would probably be beneficail too in the long run considering not only is md5 got the better encryption algorithm but also much faster.