weird session problem...

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

weird session problem...

Post 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.
Last edited by mickd on Fri Sep 23, 2005 8:03 am, edited 4 times in total.
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Post by shiznatix »

post your code. high posibility that there is somthing wrong with that
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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';


?>
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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...
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post 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.
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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';
	}
}
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

Not exactly answering your question, but what is stored in this cookie?
This login seems pretty insecure
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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?
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post 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.
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post 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...;)

Code: Select all

$pass = md5($_POST['password']);
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...
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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.
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post 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';
	}
}
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post 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...
Post Reply