Authentication Secure? (another login form =/ )

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
User avatar
Frozenlight777
Forum Commoner
Posts: 75
Joined: Wed May 28, 2008 12:59 pm

Authentication Secure? (another login form =/ )

Post by Frozenlight777 »

I know there's plenty of login authentication posts on here but I'm just looking for some feedback and opinions on how secure or not secure my script is and what I could possibly do to make it better.

Login.php

Code: Select all

<?php
 
include_once('db.php');
 
session_start();
 
// Check if user wants to login (GET info)
if(isset($_GET['try'])) {
 
    // That's nice, user wants to login. But lets check if user has filled in all information
    If(empty($_POST['username']) OR empty($_POST['password'])) {
        echo 'Please fill in all the required fields!';
    } 
    else {
        $username = addslashes($_POST['username']);
        $password = md5($_POST['password']);
        $query = mysql_query("SELECT uid FROM users WHERE username = '" . $username . "' AND passsword = '" . $password . "'") or die(mysql_error());
        
        list($user_id) = mysql_fetch_row($query);
 
        // If the user_id is empty no combination was found
        if(empty($user_id)) {
            echo 'No combination of username and password found.';
        }
        else{
            // the user_id variable doesn't seem to be empty, so a combination was found!
            // Create new session, store the user id
            $_SESSION['user_id'] = $user_id;
            header('location: profile.php');
        }       
    
    }
 
}
 
?>
<form action="login.php?try=true" method="post">
    Username: <input type="text" name="username"><br>
    <br>
    Password: <input type="password" name="password"><br>
    <br>
    <input type="submit" value="Login">
    <a href="register.php"> Register </a>
</form>


profile.php

Code: Select all

<?php
 
include_once('db.php');
 
session_start(); 
 
if(isset($_SESSION['user_id'])) {
 
        // User is logged in!
        $query = mysql_query("SELECT username FROM users WHERE UID = " . $_SESSION['user_id'] . " LIMIT 1") or die(mysql_error());
 
        list($username) = mysql_fetch_row($query);
                echo 'Hi '. $username . ', welcome to your profile!';
} 
    else {
    
    header('location: login.php');
    }
?>
I have a registration form with a captcha process, but i'll save your eyes and not post that.
I appreciate the responses.... best way to learn.
nowaydown1
Forum Contributor
Posts: 169
Joined: Sun Apr 27, 2008 1:22 am

Re: Authentication Secure? (another login form =/ )

Post by nowaydown1 »

Hi Frozen,

There's a lot of security minded folks on this forum more so than I, so keep that in mind when reading over my suggestions. Overall, the script looked alright to me. I have three suggestions for improvements:

-I noticed you were using addslashes. I think the preferred method these days is to use mysql_real_escape_string. Shiflett had an interesting write-up about why to use mysql_real_escape_string over addslashes if you're interested.
-I'm not sure if list() will initialize your $user_id variable in the event of no rows being returned. If it doesn't, and you ran this script in an environment with the register_globals directive enabled, I could potentially log in as whomever I pleased. If register_globals is off, or if list() does in fact initialize if the data set is empty, this isn't an issue.

Code: Select all

 
$query = mysql_query("SELECT username FROM users WHERE UID = " . $_SESSION['user_id'] . " LIMIT 1") or die(mysql_error());
 
This is probably okay, but I generally like to keep on the safe side and escape anything I'm putting in a query even if I think that the data should be safe. Those were the only things I noticed.
WebbieDave
Forum Contributor
Posts: 213
Joined: Sun Jul 15, 2007 7:07 am

Re: Authentication Secure? (another login form =/ )

Post by WebbieDave »

No reason to print db details with die(mysql_error()). Just print a general error message: die('An error has occured. Please contact support.') or something like that.
User avatar
lonelywolf
Forum Commoner
Posts: 28
Joined: Tue Jun 10, 2008 6:15 am

Re: Authentication Secure? (another login form =/ )

Post by lonelywolf »

Frozenlight777 wrote:$password = md5($_POST['password']);
You can enhance level security for you web by writing a new encryption function. As you know, md5 is not always safe now, because the dictionary for break md5 encryption using brute force had been enhance ^^.

You don't need to code a new encryption method :D , just using combination of some encryption methods to increase the level of security of your app.
vspin
Forum Commoner
Posts: 33
Joined: Tue Apr 29, 2008 6:31 pm

Re: Authentication Secure? (another login form =/ )

Post by vspin »

lonelywolf wrote:
Frozenlight777 wrote:$password = md5($_POST['password']);
You can enhance level security for you web by writing a new encryption function. As you know, md5 is not always safe now, because the dictionary for break md5 encryption using brute force had been enhance ^^.

You don't need to code a new encryption method :D , just using combination of some encryption methods to increase the level of security of your app.
I usually do something roughly like this:

$salt = 'h6Tg0oH3Sv';
$username = trim($_POST['userid']);
$password = sha1(strtolower(trim($_POST['pass'])) . $salt . strtolower($username));
LBmtb
Forum Newbie
Posts: 23
Joined: Wed May 14, 2008 11:14 am

Re: Authentication Secure? (another login form =/ )

Post by LBmtb »

I see a couple chances for session based attacks. To help prevent those . . .

1) regenerate the session ID when you log the person in with session_regenerate_id(). This will help prevent session fixation.
2) keep track of the user agent in the session. if the user agent changes, log them out and force them to login again:

Code: Select all

 if ($_SESSION['user_agent'] != $_SERVER['HTTP_USER_AGENT']) {
    // logout and force them to login again
}
That will help prevent session hijacking.

Good luck!
mpetrovich
Forum Commoner
Posts: 55
Joined: Fri Oct 19, 2007 2:02 am
Location: Vancouver, WA, USA

Re: Authentication Secure? (another login form =/ )

Post by mpetrovich »

I like LBmtb's thinking as a further check. The IP might be better than the User Agent. User Agents are not unique, although there is quite a variety. IPs may also not be unique. You could also combine IP and User Agent as well into the session variable.

Code: Select all

if ($_SESSION['user_agent'] != $_SERVER['REMOTE_ADDR']) {
    // logout and force them to login again
}
   //  or 
if ($_SESSION['user_check'] != $_SERVER['REMOTE_ADDR'].$_SERVER['HTTP_USER_AGENT']) {
}
User avatar
Stryks
Forum Regular
Posts: 746
Joined: Wed Jan 14, 2004 5:06 pm

Re: Authentication Secure? (another login form =/ )

Post by Stryks »

The user agent example is a good one I think. It's not unique, but it will stay static across a specific users visit.

IP addresses on the other hand may not - have a search here for using IP addresses for user tracking. It's just not a good practice to risk rejecting legitimate users for reasons that are transparent to them, and that they have no control over.
mpetrovich
Forum Commoner
Posts: 55
Joined: Fri Oct 19, 2007 2:02 am
Location: Vancouver, WA, USA

Re: Authentication Secure? (another login form =/ )

Post by mpetrovich »

IP addresses on the other hand may not (be static) - have a search here for using IP addresses for user tracking.
Good advice. Thanks. Just took the tour.
It's just not a good practice to risk rejecting legitimate users for reasons that are transparent to them, and that they have no control over.
In this case you would not really be rejecting them, unless as you pointed out, their IP changed. Although, if their IP changes, maybe we do want them to logout? Worst case is re-logging in.

We are just wanting to make sure that the Session is still with the same person. I see it can be tough. If you have a large company, they could all have the same IP and User-Agent.

Thanks for the insight.
LBmtb
Forum Newbie
Posts: 23
Joined: Wed May 14, 2008 11:14 am

Re: Authentication Secure? (another login form =/ )

Post by LBmtb »

What if they're in a university or large company and are on a laptop. The they could easily switch IP because they moved closer to a different access point.
mpetrovich
Forum Commoner
Posts: 55
Joined: Fri Oct 19, 2007 2:02 am
Location: Vancouver, WA, USA

Re: Authentication Secure? (another login form =/ )

Post by mpetrovich »

What if they're in a university or large company and are on a laptop. The they could easily switch IP because they moved closer to a different access point.
I had not experienced that. Good point. In thinking through my various applications, I am thinking I may still want them to re-log in. It would be equivalent to a session time-out. If it was a frequent change, then that certainly would not be an acceptable solution.
LBmtb
Forum Newbie
Posts: 23
Joined: Wed May 14, 2008 11:14 am

Re: Authentication Secure? (another login form =/ )

Post by LBmtb »

Maybe track how many times that happens (make them login again because the IP changes). I'd be interested to know how often that would happen.
sbarros
Forum Newbie
Posts: 1
Joined: Fri Aug 29, 2008 2:48 pm

Re: Authentication Secure? (another login form =/ )

Post by sbarros »

Hi Frozen,
Please, could you post the code db.php?
In my phpinfo() register_globals is off.
I need protect a directory with photos album
Thanks in advance
Post Reply