session help, check if exist

[adammc]

Hi folks,

I was wondering if someone could ensure that I am doing this the right way?

I have a login page with a form asking for username, password and state. The username and pass are validated against my database to ensure they exist, if not they are sent back to the login form page. I then want to check on my inside pages wether the user is logged in or not, if not redirect them back to the login form.

login form

<form action = "authenticate.php" method = "post">


  <table>
    <TR><TD><font class="maintext">Username:</font> </TD><TD><INPUT TYPE="text" NAME="username" SIZE=20></TD></TR>

<TR><TD><font class="maintext">Password:</font> </TD><TD><INPUT TYPE="text" NAME="password" SIZE=20></TD></TR>

<tr>
        <td valign="center"><font class="maintext">State:</font></td>
        <td valign="center">
<SELECT name="state" class="textbox"> 
	<OPTION value="">please select</OPTION>
	<OPTION value="ACT">ACT</OPTION>
	<OPTION value="New South Wales">New South Wales</OPTION>
	<OPTION value="Northern Territory">Northern Territory</OPTION>
	<OPTION value="Queensland">Queensland</OPTION>
	<OPTION value="South Australia">South Australia</OPTION>
	<OPTION value="Tasmania">Tasmania</OPTION>
	<OPTION value="Victoria">Victoria</OPTION>
	<OPTION value="Western Australia">Western Australia</OPTION>


</SELECT>
</tr> 

<tr><td>    <input type="Submit" value="Submit"></td></tr>
</table>


</form>

authenticate.php

<?php
session_start();

session_register("userinfo");
if((!$_POST['username']) or (!$_POST['password']))
{ header("Location:$HTTP_REFERER"); exit(); }

$username=$_POST['username'];
$password=$_POST['password'];
$state=$_POST['state'];

 session_register("state");
 session_register("username");
 session_register("password");

$conn = @mysql_connect("localhost","user","pass") or die("Err:Conn");
$rs = @mysql_select_db("database",$conn) or die("Err:Db");

$sql = "select * from members where username='$username' and password = '$password' ";
$rs = mysql_query($sql, $conn) or die("Err:Query");

$match = mysql_numrows($rs);

if ($match != 0) {
$userinfo=mysql_fetch_array($rs);
mysql_close($conn);
header("Location:test-login-success.php");

exit();
} 

else {
header("Location:login.html");
exit();
}

?>

code to put on my member pages

<?php 

session_start();

if ($_SESSION["username"]=="") {
header('Location: login.html');

if ($_SESSION["password"]=="") {
header('Location: login.html');

if ($_SESSION["state"]=="") {
header('Location: login.html');

}else{ 

?>


Welcome back <? echo $_SESSION['username'];?>, login was successful!
<br> you are from <? echo $_SESSION['state'];?>

<br><br>sdfdsfdsfsdfsdfsdfsdfs<br><br>

 <A HREF="test-login-destroy.php">logout</A>


<? 
}

?>

Is this code going to do the job, is it secure enough?
Any help would be GREATLY appreciated.

Regards
Adam

[bpat1434]

1.) mysql_numrows is a deprecated function. Use [man]mysql_numrows[/man] instead.
2.) Password input is "text", not of the "password" type, so the password will be visible.
3.) If $SESSION (or $HTTP_SESSION_VARS for PHP 4.0.6 or less) is used, assign values to $SESSION. For example: $SESSION['var'] = 'ABC';
4.) Your check to see if they're logged in can be easily put into one if() statement:

if(empty($_SESSION) || empty($_SESSION['username']) || empty($_SESSION['password']) || empty($_SESSION['state']))
{
    header("Location: login.html");
    exit;
}

// No need for an "else{}" declaration because 
// if it catches in the if() statement, the script will 
// exit and not execute ;)

Other than your password being plain-text (both in the DB and in the form) and you storing it in the session, there isn't too much security to worry about, except SQL injection. You should use [man]sprintf/man to help secure the what you put in your query, and [man]mysql_real_escape_string/man to ensure that no extra commands are added 😉

[ahundiak]

First off session_register only works if register_globals is ON which makes your script inherently less secure. And besides, just using $_SESSION['name'] = $value; is easier and clearer then using session_register. So turn globals off.
http://us3.php.net/manual/en/function.session-register.php

I did not actually run the script but it appears to me that you register your variables before checking the password and never unregister them if the password is incorrect. Try an unsuccesful login then try going directly to your members page. I think it might let you in.

Passwords are usually not stored in ther original plain text format. Scramble them with md5($password) so if someone does get to your database they won't automatically get everyone's password.

I would also simplify things by just having a boolean $isAuthenticated variable. Your check for proper login then becomes if (!$_SESSION('isAuthenticated')) redirect to login page.

And don't store passwords in session data. Session data files are usually easy to view. In fact, in some cases you can see them right over the web.