Account Login Review

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
Daz
Forum Newbie
Posts: 18
Joined: Thu Mar 19, 2009 2:12 am

Account Login Review

Post by Daz »

I'm a coder new to the realm of websites. I'm working on a personal website and was hoping I could ask the pros here if my understanding of a "secure" login process is decent. I hope you don't mind that I'll be using a combination of pseudo and php ;)

login.php
Login form has two fields (username/password), as well as a captcha

[/login.php?do=login]

Code: Select all

session_start();
$pass = mysql_real_escape_string($_POST('password'));
$pass = sha1( trim($pass) . trim($salt) );
$user = trim(mysql_real_escape_string($_POST('username')));
$sql = "SELECT * FROM users WHERE `username` = '$user' AND `password` = '$pass'  LIMIT 1";
$result = $db->getrow($sql);
 
IF ( $result)   {
   session_regenerate_id();
   $_SESSION('client_hash') = sha1( $_SERVER['HTTP_USER_AGENT'] . $_SERVER['REMOTE_ADDR'] );
   //Create session variables
}   ELSE   {
   session_destroy();
}
Now, on every page the requires the user account to access make sure that:
  • $_SESSION('client_hash') = sha1( $_SERVER['HTTP_USER_AGENT'] . $_SERVER['REMOTE_ADDR'] )
Thanks in advance for your criticisms and advice!
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: Account Login Review

Post by kaisellgren »

I do not think you have any excuses for trimming the password, do you? Sometimes I use a space in front of my passwords due to an inside joke long ago. Thus, my login would always fail on your site.

Trimming the salt does not make sense esp. since you are the one who made it in the first place.

Same applies to the username... surprise...

You do realize that people behind dynamic IP proxies may never be able to use your site?

By the way, $_SESSION[] works better than $_SESSION() me thinks. 8)
Post Reply