Page 1 of 1
weird session problem...
Posted: Wed Sep 21, 2005 5:07 am
by mickd
hi, im getting this really weird bug when logging in.
for some reason, the first attempt to log in the session doesnt get set but the second time it does.... ive echoed the value of the variable that im going to set the session's value as and its echoed value is correct.
anyone know of any reason why this would happen?
thanks, any help appriciated.
Posted: Wed Sep 21, 2005 5:15 am
by shiznatix
post your code. high posibility that there is somthing wrong with that
Posted: Wed Sep 21, 2005 5:24 am
by mickd
Code: Select all
<?php
@session_start();
if(isset($_POST['login'])) {
if(preg_match('/^[A-Za-z0-9]{4,15}$/', $_POST['username']) && preg_match('/^[A-Za-z0-9]{4,15}$/', $_POST['password'])) {
mysql_connect("localhost", "XXXXX", "XXXXX") or die("Could not connect to the database.");
mysql_select_db("XXXXXX");
$pass = md5($_POST['password']);
$login_check = mysql_query("SELECT username, password FROM accounts WHERE username='" . $_POST['username'] . "' AND password='$pass'");
if(mysql_num_rows($login_check) == '1') {
$check_status = mysql_query("SELECT * FROM accounts WHERE username='" . $_POST['username'] . "'");
$status_assoc = mysql_fetch_assoc($check_status);
if($status_assoc[activated] == '1') {
if($status_assoc[suspension_time] == '0') {
if($status_assoc[banned] == '0') {
while($login_session_check != '1') {
$login_session_number = mt_rand();
$login_session = md5($login_session_number);
$login_session_query = mysql_query("SELECT is_login_session FROM accounts WHERE is_login_session='$login_session'");
if(mysql_num_rows($login_session_query) == '0') {
$login_session_check = '1';
mysql_query("UPDATE accounts SET is_login_session='$login_session' WHERE username='" . $_POST['username'] . "'");
$_SESSION['is_login_session'] = $login_session;
}
}
while($login_cookie_check != '1') {
$login_cookie_number = mt_rand();
$login_cookie = md5($login_cookie_number);
$login_cookie_query = mysql_query("SELECT is_login_cookie FROM accounts WHERE is_login_cookie='$login_cookie'");
if(mysql_num_rows($login_cookie_query) == '0') {
$login_cookie_check = '1';
mysql_query("UPDATE accounts SET is_login_cookie='$login_cookie' WHERE username='" . $_POST['username'] . "'");
setcookie("is_login_cookie", $login_cookie, time()+3600, "/", ".XXXXXX");
}
}
header("location:http://www.XXXXX.com/XXX.php?test=$login_session");
} else {
$title = 'XXXX';
include 'header.php';
echo error(banned);
include 'footer.php';
exit;
}
} else {
$title = 'XXXX';
include 'header.php';
echo error(suspended);
include 'footer.php';
exit;
}
} else {
$title = 'XXXX';
include 'header.php';
echo 'Please activate this account first.';
include 'footer.php';
exit;
}
} else {
$title = 'XXXX';
include 'header.php';
echo 'Login failed.';
include 'footer.php';
exit;
}
} else {
$title = 'XXXX';
include 'header.php';
echo 'Login failed.';
include 'footer.php';
exit;
}
}
$title = 'XXXX';
include 'header.php';
if(is_login() == 'FALSE') {
echo 'Your session has expired.';
include 'footer.php';
exit;
}
?>
REMOVED CODE HERE
<?php
include 'footer.php';
?>
Posted: Thu Sep 22, 2005 5:32 am
by mickd
hmm noone knows why its happening?
seems weird that the session doesnt get set the first time but the second time it does...
Posted: Thu Sep 22, 2005 6:12 am
by Jenk
What's the code for the function is_login() ?
Do you really need to surpress any errors from session_start() ? I didn't think it would generate any.
Posted: Thu Sep 22, 2005 6:57 am
by mickd
i tried it without before too, didnt make any difference
code for is_login which is in a config.php file which is included inside header.php
Code: Select all
function is_login()
{
$select_check_login = mysql_query("SELECT is_login_cookie, is_login_session FROM accounts WHERE is_login_cookie='" . $_COOKIE['is_login_cookie'] . "' AND is_login_session='" . $_SESSION['is_login_session'] . "'");
$check_login = mysql_num_rows($select_check_login);
if($check_login == '1' && isset($_COOKIE['is_login_cookie']) && isset($_SESSION['is_login_session'])) {
return 'TRUE';
} else {
return 'FALSE';
}
}
Posted: Thu Sep 22, 2005 7:28 am
by John Cartwright
Not exactly answering your question, but what is stored in this cookie?
This login seems pretty insecure
Posted: Thu Sep 22, 2005 7:49 am
by mickd
its a random number md5 dashed which i store the value in the cookie and in my database in a field called is_login_cookie and i also do the same with a session (different random md5 dashed value stored in a field called is_login_session), then i compare the values of the cookie and session thats set on the client and see if both the values equal to the same values in 1 row. i used a loop to make sure that in 1 field you cant have 2 of the same number md5 dashed stored so if by chance it generated the same itll generate another one until it gets one not previously stored.
btw how would you recommend making it a more secue loggin?
Posted: Thu Sep 22, 2005 8:06 am
by Jenk
Sanitise the cookie input for a start, cookie 'spoofing' (for lack of actual term, I can't remember it) is quite easy, so if they changed it to an extension of your SQL, they can gain access to your site wihout the need to login.
The problem looks like you might be trying to access session data before it has been set, I'm not totally convinced myself though.
Thus on the second attempt/page load, the session data is there from the first attempt thus login works.
Posted: Thu Sep 22, 2005 8:36 am
by mickd
i thought about that myself too but then i found out that you can login on the first attempt but only if you first visit the page that has that code shown above. by the way the header("location: blah"); in the code above sends the user back to the same page but after it sends them there the very top shouldnt be executed because it should only be executed if the login submit button was submited.
edit: going to go sleep now, will be awake in 6hrs and 30 min from now and ill check replies then.
btw, thanks for your help so far guys.
Posted: Thu Sep 22, 2005 9:26 am
by Maugrim_The_Reaper
Implement a challenge/response login process. Being the third? time mentioned by me on the forums I'm thinking of posting the code for one just so I can reference it, and show how simple it is...
Basically - you hash the password *clientside* in a string with username, and a random challenge (a string generated, stored to DB, and outputted as hidden formfield). This new hash is called the response. The idea is that you only send the username and the response - but not the plain text password - which is insecure if someone listens to your network traffic...
Your code is a little convoluted. You have the right idea, just getting the steps needs a little work.
Just a few quick observations, in no particular order or purpose...
Code: Select all
if(preg_match('/^[A-Za-z0-9]{4,15}$/', $_POST['username']) && preg_match('/^[A-Za-z0-9]{4,15}$/', $_POST['password'])) {
I take it you're checking the password is alphanumeric? In which case look up the function ctype_alnum() in the manual. I love all ctype functions...
sha1 is more secure than md5. SHA256 even better - see feyd's implementation. Nothing actually wrong here though...just nitpicking...
Code: Select all
$login_check = mysql_query("SELECT username, password FROM accounts WHERE username='" . $_POST['username'] . "' AND password='$pass'");
if(mysql_num_rows($login_check) == '1') {
$check_status = mysql_query("SELECT * FROM accounts WHERE username='" . $_POST['username'] . "'");
You run two sql queries - each of which basically gets user info. Could combine into just one query...
Code: Select all
$status_assoc = mysql_fetch_assoc($check_status);
if($status_assoc[activated] == '1') {
if($status_assoc[suspension_time] == '0') {
if($status_assoc[banned] == '0') {
Looks fine...may get a little confusing if you need to add extra checks and such...
Code: Select all
$login_session_number = mt_rand();
Look up uniqid() in manual - may be more suitable.
Code: Select all
while($login_session_check != '1') {
$login_session_number = mt_rand();
$login_session = md5($login_session_number);
$login_session_query = mysql_query("SELECT is_login_session FROM accounts WHERE is_login_session='$login_session'");
if(mysql_num_rows($login_session_query) == '0') {
$login_session_check = '1';
mysql_query("UPDATE accounts SET is_login_session='$login_session' WHERE username='" . $_POST['username'] . "'");
$_SESSION['is_login_session'] = $login_session;
}
}
while($login_cookie_check != '1') {
$login_cookie_number = mt_rand();
$login_cookie = md5($login_cookie_number);
$login_cookie_query = mysql_query("SELECT is_login_cookie FROM accounts WHERE is_login_cookie='$login_cookie'");
if(mysql_num_rows($login_cookie_query) == '0') {
$login_cookie_check = '1';
mysql_query("UPDATE accounts SET is_login_cookie='$login_cookie' WHERE username='" . $_POST['username'] . "'");
setcookie("is_login_cookie", $login_cookie, time()+3600, "/", ".XXXXXX");
}
Might be wrong on this - nice and complex

- anything wrong with just storing a user's id and username in the cookie side? It's a question for anyone else... May be reasons to keep an id/username off the cookie of course.
Has to be somthing in here causing that first failure. I've seen it a few times, and it's always vanished after a more segegated approach (splitting things up more concretely).
For other security measures -
http://shiflett.org/. Chris has some really good articles on his site. Its just one source...
Posted: Fri Sep 23, 2005 1:02 am
by mickd
thanks for the tips, by the way, when you say *clientside*
Maugrim_The_Reaper wrote:Basically - you hash the password *clientside* in a string with username, and a random challenge (a string generated, stored to DB, and outputted as hidden formfield). This new hash is called the response. The idea is that you only send the username and the response - but not the plain text password - which is insecure if someone listens to your network traffic...
do you mean store the username, id and the responce as a string in a cookie?
also if you put the responce in a hidden form type when the use visits the page, wouldnt they be able to view the value of the responce in the view/source or does that not matter?
thanks.
Posted: Fri Sep 23, 2005 7:16 am
by mickd
i rewrote the code using some of the advice that you said, the 2 login thing is gone
any other comments on the code now?
thanks
Code: Select all
<?php
if(isset($_POST['login'])) {
$token = sha1(md5(uniqid(rand())));
if(preg_match('/^[A-Za-z0-9]{4,15}$/', $_POST['username']) && preg_match('/^[A-Za-z0-9]{4,15}$/', $_POST['password'])) {
@mysql_connect("localhost", "XXX", "XXX") or die("Could not connect to the database.");
mysql_select_db("XXX");
$pass = sha1($_POST['password']);
$check_status = mysql_query("SELECT * FROM accounts WHERE username='" . $_POST['username'] . "' AND password='$pass'");
if(mysql_num_rows($check_status) == '1') {
$status_assoc = mysql_fetch_assoc($check_status);
if($status_assoc[activated] == '1') {
if($status_assoc[suspension_time] == '0') {
if($status_assoc[banned] == '0') {
mysql_query("UPDATE accounts SET token='$token' WHERE username='" . $_POST['username'] . "'");
$identification = '' . $_POST['username'] . '-' . $token . '';
setcookie("identification", $identification, time()+3600, "/", ".XXX.com");
header("location:http://www.XXX.com/XXX.php");
die();
} else {
$title = 'XXX';
include 'header.php';
echo error(banned);
include 'footer.php';
exit;
}
} else {
$title = 'XXX';
include 'header.php';
echo error(suspended);
include 'footer.php';
exit;
}
} else {
$title = 'XXX';
include 'header.php';
echo 'Please activate this account first.';
include 'footer.php';
exit;
}
} else {
$title = 'XXX';
include 'header.php';
echo 'Login failed.';
include 'footer.php';
exit;
}
} else {
$title = 'XXX';
include 'header.php';
echo 'Login failed.';
include 'footer.php';
exit;
}
}
$title = 'XXX';
include 'header.php';
if(is_login() == 'FALSE') {
echo 'Your session has expired.';
include 'footer.php';
exit;
}
?>
CODE REMOVED HERE
<?php
include 'footer.php';
?>
changed all encrpytion to sha1 too.
is_login is
Code: Select all
function is_login()
{
$identification = explode("-", $_COOKIE['identification']);
$username = $identification[0];
$token = $identification[1];
if(preg_match('/^[A-Za-z0-9]{40}$/', $token)) {
$select_check_login = mysql_query("SELECT username, token FROM accounts WHERE username='$username' AND token='$token'");
$check_login = mysql_num_rows($select_check_login);
if($check_login == '1' && isset($_COOKIE['identification'])) {
return 'TRUE';
} else {
return 'FALSE';
}
} else {
return 'FALSE';
}
}
Posted: Fri Sep 23, 2005 10:47 am
by Maugrim_The_Reaper
Other comments:
Just one - you only need to use sha1 or md5 - not both, think you have sha1(md5(string)) in there.
do you mean store the username, id and the responce as a string in a cookie?
also if you put the responce in a hidden form type when the use visits the page, wouldnt they be able to view the value of the responce in the view/source or does that not matter?
Don't worry about implementing this - I'll post something on it later. Client (the user and browser) can see the challenge and the response - so can anyone else. But your hashed reponse keeps changing from login to login making guessing the password near impossible. Compare this to just sending the plain text password...
Glad its now working... Next up - input filtering and escaping...