Login script security

[j.smith1981]

I am a bit concerned about using this code here, is there anyway of better authenticating a user?

 

Just concerned about the usual ones like session hijacking, not really SQL injection (as I have put mysql_real_escape_string function below), but also XSS attacks etc, how would I go about improving this script?

 

<?php

ini_set('display_errors', 0);

require_once 'header.html';

require_once 'db.functions.php';

require_once 'config.php';

$message = 'To login please enter your user information below';

$database = dbConnect($host, $username, $password, $database); // should output 1 or nothing at all!

if($database == true)
{

if(array_key_exists('submit',$_POST))
{

	if($_POST['username'] != '' && $_POST['password'] != '')
	{
		// carry on with logging in process:
		$username = mysql_real_escape_string($_POST['username']);
		$password = mysql_real_escape_string(sha1($_POST['password']));

		$sql = "SELECT username, password FROM users WHERE username = '$username' AND password = '$password'";
		$result = mysql_query($sql);

		if($result)
		{

			// query worked now try and log them in:
			$result = mysql_num_rows($result);

			if($result == 1)
			{

				// do a query to retrieve users information:
				$sql = "SELECT username, password FROM users WHERE username = '$username' AND password = '$password'";
				$result = mysql_query($sql);
				$user = mysql_fetch_array($result);

				session_start();
				$_SESSION['login'] = 1;
				$_SESSION['username'] = $user[0];
				header("location: index.php");

			} else {

				$message = 'You entered a wrong username or password please try again';

			}

		} else {

			$message = 'Could not query database for some reason, please try again later';

		}

	} else {
		// output an error message to the user:
		$message = 'You did not enter all the required fields to login, please retry';
	}

}

} else {

$message = 'Could not connect to database, please try again later';

}

printf("<p>%s</p>",$message);

echo <<<USER_LOGIN

<form id="login" name="login" method="post" action="{$_SERVER['PHP_SELF']}">

<p><label for="username">Username: </label> <input type="text" id="username" name="username" value="{$username}" /> </p>

<p><label for="password">Password: </label> <input type="password" id="password" name="password" value="" /> </p>

<input type="submit" id="submit" name="submit" value="Login" />

</form>

USER_LOGIN;

require_once 'footer.html';

 

Works quite well to be honest, but just wondered if there's anything I can do to improve my example blogs security, any suggestions are helpful,

Jez.

[Muddy_Funster]

Biggest security hole I see is that you are storing the password in plain text, and pulling that password out of the database during the login.  all you need to get back from the database is a single field (username, id etc.) during the login so that you can establish a user exists matching the login credentials given by the user.

 

As for further enhancements...out with my scope to comment through experiance, but some speculation I can offer is:

1. add a field to the database for the IP scope of the user logging in (ip address minus last three possible digits) and have a comparison on that.  if the user loggs in from another location - ie school, office, university - than is held on in the database, you can have an email sent to the registered address stating this (or block it from being able to access at all - but that's a smidge extreme).  You can also generate custom session variables, again that you can store in the database, based on other client information at time of login, to add extra security to the sessions.

 

Hope that at least helps a little

[PFMaBiSmAd]

you are storing the password in plain text

 

No, he's not. The password is a sha1() hash.

[codefossa]

The only way you can get around session hijacking is if you have secure encrypted connections (https:).  Since you most likely wont be dealing with that, just attempt to make it as secure as you can.  Personally, I store the username and password inside of the session or cookies, and on each visit check them to the database.  If in the database a row with the username and password exists, then they're logged in.  Also, I would use something like sha1(md5($password)) just to help with the security, as there's a lot of sites storing huge databases of encrypted strings to decrypt passwords.  You may want to even go with something a little stronger, like adding a randomized salt to it, and making sure you store that in the database to use the same string each time.

[Muddy_Funster]

you are storing the password in plain text

 

No, he's not. The password is a sha1() hash.

Fair point there, I missed that  appologies.

[j.smith1981]

you are storing the password in plain text

 

No, he's not. The password is a sha1() hash.

 

No thats fine, I have done that quite allot on other peoples forums lol, thanks for noticing though, much appreciated

 

But on the subject of doing say a salt, is that just an encrypted set of characters into a password? Not sure exactly how theoretically that should work, can someone help to get my head around salts? I have of course seen them all the time on the web in general but never actually bothered to implement them, because I have never totally understood what they are.

 

Basically as a general jist of myself when I am developing something, I like to fully appreciate what I am doing and why I am doing it (I know its for security but just lack the theory of understanding how its securing what it is I am creating), if I dont therefore understand what I am doing, I dont do it.

 

Any help agains really appreciated, and thank you for replying so quickly,

Jez.

[xylex]

A salt in this case would just be a random set of characters you would append to their password before you did the sha1 hashing.  The idea there is that even if someone got the password value from the DB, even if they tried to brute force a short password, or had a rainbow table of matches, they wouldn't be able to use it to get the original password because of the salt.  Just make sure you use a long enough and random enough string to do this (like don't make your salt 'salt', which I have seen).

 

Two other security pieces for you:

 

• Add in is a call to session_regenerate_id() after your session_start() call, this will take care of session fixation attacks.
  
• Don't directly echo $username; echo htmlspecialchars($username, ENT_QUOTES);  Otherwise, you're putting an XSS vulnerability on your page.
  

[j.smith1981]

Thats an excellent reply!

 

Makes perfect sense really, I mean with fixation attacks, I bet that's because most hackers (or as I call them crackers), are people that know how PHP comes up with the SESS_ID, right?

 

Please correct me if I am wrong.

 

If I was to output the values of say a username, I would use a normal echo would I? I am just a bit confused about the last tip you gave me.

 

Thanks though much appreciated and I look forward to your reply,

Jez.

 

[j.smith1981]

Ok this is what I have amended:

 

<?php
session_name("jeremysmith_blog");
session_start();

// init the logout script?
if(array_key_exists('action',$_GET))
{
if($_GET['action'] == 'logout')
{
	// log user out of the system:
	session_unset($_SESSION['login']);
	session_unset($_SESSION['username']);
	session_unset($_SESSION['type']);
	session_destroy();
}
}

if(array_key_exists('submit',$_POST))
{
print_r($_POST);
}

ini_set('display_errors',0);

require_once 'header.html';

require_once 'db.functions.php';

require_once 'config.php';

$database = dbConnect($host, $username, $password, $database); // should output 1 or nothing at all!

if($database == true)
{
// now connected?
// carry on with logic of outputting the blog contents:
$result = entries("SELECT * FROM entries");

printf("<table>");

while($row = mysql_fetch_array($result))
{
	printf("
	<tr>
	<td>%s</td>	<td>%s</td>
	</tr>

	<tr>
	<td colspan=\"2\">%s</td>
	</tr>
	", $row[2], $row[4], $row[3]);
}

printf("</table>");

printf("\n\n");

	if(array_key_exists('login',$_SESSION))
{
	if($_SESSION['login'] == 1) // change this to correspond with session on the login.php script
	{
		session_regenerate_id();

		printf("<p>Welcome %s</p>

		<p>You may post entries using this form:</p>

		<!-- <form id=\"add\" name=\"add\" action=\"\" method=\"post\"> -->
		<form id=\"add\" name=\"add\" action=\"index.php\" method=\"post\">

		<input type=\"text\" id=\"\" name=\"\" value=\"\" />

		</form>

		<p>To logout, click <a href=\"index.php?action=logout\">here</a></p>

		",$username);

		print_r($_SESSION);
	}

} else {
	printf("<p>You are not logged in, please click <a href=\"login.php\">here</a> to login.</p>");
}


} else {

printf("\n<p id=\"error\">Could not connect to database, please try again later.</p>");

}


printf("\n"); // just for output format!
require_once 'footer.html';

 

The part where:

 

session_regenerate_id();

 

It is just duplicating the session file in the sessions folder, when I go to logout on a test account I have made, its not removing 1 of the 2 files, how would I go about doing this?

 

Just thinking for garbage clearing you know?

 

I will add in a session checker in the database, I mean I could just go for that outright could I? I mean I would always theoretically have to create a cookie on the users browser, to check against the mysql table or the filesystem, but would mysql custom session handlers be better for this?

 

Would that also mean that I would avoid creating a file in the sessions folder for PHP?

 

In anycase, is there any good tutorials for timing out a session? Surely I would have to put either a timestamp into the session cookie wouldn't I? Then if thats offset by a certain time, but how would I best go about doing this?

 

Is there any good tutorials on this aspect of security?

 

Thank you so much for your help,

Jeremy.

[j.smith1981]

Got this off php.net:

 

<?php
session_regenerate_id();
session_destroy();
unset($_SESSION);
session_start();

 

I am having problems, that obviously, when I refresh the page, it logs the user out, just can't work out the theory in how to get it to keep that login, hmm would be interesting.

 

I have added this:

 

<?php

session_regenerate_id();
session_destroy();
unset($_SESSION);
session_name("jeremysmith_blog");
session_start();

 

Any thoughts?

 

Thanks again,

Jez.