[SOLVED] Effective 'Logged In' Checker?

[wintallo]

I'm writing my own login script and have the following bit of code to check whether or not the users are logged or not.

The script works, I'm just wondering if there are any security problems with it.

 

if ( isset($_SESSION['username']) ) {
$username = $_SESSION['username'];
$password = $_SESSION['password'];
$query = "SELECT * FROM members WHERE username = '".addslashes($username)."'";
$result = mysql_query($query);
$memberinfo = mysql_fetch_array($result);
if ( $password != $memberinfo['password'] ) {
	header("Location: login.php");
}
} else {
header("Location: login.php");
}

Thanks in advance 

 

-Joel

http://www.wintallo.com

[redarrow]

mysql_real_escape_string($username);

Is more powerfull then addslashes ok use that.

[obsidian]

Within that code itself you should be fine. You really don't even have to double check the password, and I would recommend strongly against storing the actual password in a session variable like that. Once they have logged in, you've already checked their password, so there's no reason to keep it. All that really serves to do is if the session is ever hijacked, it gives the hijacker access to the account directly through the password. You could simply check to see if your session "username" variable is set and run from there.

[redarrow]

 

 

I am sure that more safer ?

<?php session_start();

if ( isset($_SESSION['username']) || ($_SESSION['password'])) {
$query = "SELECT * FROM members WHERE username = '".mysql_escape_string($_SESSION['username'])."'";
$result = mysql_query($query);
$memberinfo = mysql_fetch_array($result);
if ( $password != $memberinfo['password'] ) {
	header("Location: login.php");
}
} else {
header("Location: login.php");
}
?>

[wintallo]

redarrow:

 

Would I have to use the stripslashes when outputting anything that had mysql_real_escape_string used on it or a different command?

 

-Joel

http://www.wintallo.com

[redarrow]

i only use the command addslashes and above for inserting information nothink else i only added the above statement becouse you used addslashes ok.

 

In realty you should use that command on registration ok and nothink here ok.

 

with the registration using the above method the code would be this

<?php session_start();

if ( isset($_SESSION['username']) || ($_SESSION['password'])) {
$query = "SELECT * FROM members WHERE username = '".$_SESSION['username']"'";
$result = mysql_query($query);
$memberinfo = mysql_fetch_array($result);
if ( $password != $memberinfo['password'] ) {
	header("Location: login.php");
}
} else {
header("Location: login.php");
}
?>

 

No one should ever be able to put slashes in the database ok

so change your insert code for regerstartion and ur be fine if you use mysql_real_escape($varable_names_for_any_insert);

[wintallo]

Obsidian:

 

I changed around the code so I doesn't do anything with the password. I added $_SESSION['loggedin'] (it's set to TRUE when the user logs in) so that on protected pages, if it's false, I doesn't redirect. Instead would display a message telling the user to log in. If the $_SESSION['loggedin'] is set to TRUE it displays the normal contents of the page. Does the $_SESSION['loggedin'] again compromise the security of this script.

 

new code

if ( isset($_SESSION['username']) ) {
$username = $_SESSION['username'];
$query = "SELECT * FROM members WHERE username = '".addslashes($username)."'";
$result = mysql_query($query);
$check = mysql_num_rows($result);
if ( $check == 0 ) {
	$message = "You must be logged in to view the contents of this page. Click <a href=\"login.php\">here</a> to login.";
	$_SESSION['loggedin'] = false;
}
} else {
$message = "You must be logged in to view the contents of this page. Click <a href=\"login.php\">here</a> to login.";
$_SESSION['loggedin'] = false;
}

 

-Joel

http://www.wintallo.com