Please critique this login script!

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
DevonL
Forum Newbie
Posts: 6
Joined: Sat Feb 11, 2012 6:13 pm

Please critique this login script!

Post by DevonL »

Code: Select all

<?
include "config.php";
include "functions.php";
session_start();
	
$t = time()+3600;
$datetime = date("Y-m-d H:i:s", $t);
$ip_address = $_SERVER['REMOTE_ADDR'];
	
if (!$PHP_AUTH_USER) {
	header('WWW-Authenticate: Basic realm="Control Panel"'); 
} 
else {
	$password = crypt($PHP_AUTH_PW);															// Encrypt the inputed password for comparison
	$query = sprintf("SELECT * FROM users WHERE username='%s' AND password='%s'",
	mysql_escape_strings($PHP_AUTH_USER),
	mysql_escape_strings($password));

	$result = mysql_query($query);
	$row = mysql_fetch_array($result);

	if (mysql_num_rows($result) != "1") { // No user or pass found - incorrect entry
		error_msg(1);
		$err = 1;							
	}
	elseif (mysql_num_rows($result)) {				// User was found
		$_SESSION['admin_name'] = $PHP_AUTH_USER;	// Set session name to username
		$crt = 1;									// Allow into control panel
	}
	else {
		error_msg(1); 
		$err = 1;
	}
}
if ($crt) { 
	header("Location: home.php");
}
?>
Please let me know how secure this will be in it's current state! Thanks guys :)

EDIT: Changed the script slightly.
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Please critique this login script!

Post by social_experiment »

DevonL wrote:Please let me know how secure this will be in it's current state!
With this you only really have to check for a possible SQL injection; mysql_escape_strings() is not an escape function (there is mysql_escape_string() within the php manual but it has been deprecated). If it's a custom function i retract my statement but why not use mysql_real_escape_string().

There is no checking of the data that you receive; another injection point

You should also regenerate the session once a user has been successfully logged in; session_regenerated_id() should help with this.

To make passing of information more secure use a secure connection to login the users.

I'm not sure if this falls within the parameters of 'secure' (regarding the login form itself) but you could also take a look at hashing with some other hash function and use an algorithm with it. Search the forum as this topic has been covered lots of times in the last few years;
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
DevonL
Forum Newbie
Posts: 6
Joined: Sat Feb 11, 2012 6:13 pm

Re: Please critique this login script!

Post by DevonL »

I'll definitely search for using other hash functions.

That was indeed supposed to be mysql_escale_string() as opposed to strings - a typo when I was tired, thanks for pointing that out though. I'll definitely read up session_regenerated_id(); - I haven't worked with PHP in some years, so things have seemed to have changed quite a bit!

I appreciate the information, thank you!
User avatar
Celauran
Moderator
Posts: 6427
Joined: Tue Nov 09, 2010 2:39 pm
Location: Montreal, Canada

Re: Please critique this login script!

Post by Celauran »

No need for another hashing function as such, just consider using Blowfish with crypt().
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Please critique this login script!

Post by social_experiment »

http://php.ss23.geek.nz/2011/01/12/Using-crypt.html
Here is an article you can read on using the function
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Please critique this login script!

Post by social_experiment »

may2hoo wrote:The fact is your original writing abilities has inspired me to get my own site now
Thanks but i cannot claim any kodus on the article; i found it through Google :)
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
Post Reply