• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 1444
  • Last Modified:

PHP secure login code

Hi,
Apology's if the answer to this is found on another post, not found it so far.
I am looking to implement a system which includes a login portal, for users (stored in db) to login using PHP sessions.
Below is the code that i have used, which works, but the security aspect is concerning me.
Could you let me know if this is a secure way to implement this, and if i can improve it in any way?

thanks in advance
All files include check_login.php:
<?php
        session_start();
        if (!isset($_SESSION['user_id'])){
                include("login.php");
                exit();
        }
?>
 
 
 
login.php:
<html>
<head>
</head>
<body>
<form action="login_submit.php" method="POST">
Username: <input type="text" name="username">
Password: <input type="password" name="password">
</form>
</body>
</html>
 
 
 
login_submit.php
<?php
        include("includes/db_connect.php");
 
        $username=$_POST['username'];
        $password=$_POST['password'];
 
        $qryGetUser="SELECT * FROM user WHERE username='$username' LIMIT 1";
        $rsGetUser=mysql_query($qryGetUser) or die(mysql_error());
 
        //USERNAME CORRECT?
        if (mysql_num_rows($rsGetUser)==0){
                echo "Username or password incorrect.";
                exit();
        }
        $rowUser=mysql_fetch_array($rsGetUser);
 
        //PASSWORD CORRECT?
        if ($rowUser['password']!=md5($password)){
                echo "Username or password incorrect.";
 
                //LOCK ACCOUNT IF THIS IS THIRD ATTEMPT
                if ($rowUser['failed_attempts']>1){
                        $qryUpdateUser="UPDATE user SET failed_attempts=failed_attempts+1, locked='1' WHERE user_id='".$rowUser['user_id']."'";
                }
                //ELSE INCREMENT FAILED ATTEMPTS
                else{
                        $qryUpdateUser="UPDATE user SET failed_attempts=failed_attempts+1 WHERE user_id='".$rowUser['user_id']."'";
                }
                $rsUpdateUser=mysql_query($qryUpdateUser) or die(mysql_error());
                exit();
        }
 //IF ACCOUNT LOCKED
        if ($rowUser['locked']=="1"){
                $qryUpdateUser="UPDATE user SET failed_attempts=failed_attempts+1 WHERE user_id='".$rowUser['user_id']."'";
                $rsUpdateUser=mysql_query($qryUpdateUser) or die(mysql_error());
                echo "<b>Error: </b>Your account has been disabled.<br/>Please contact your systems administrator.";
                exit();
        }
 
        //LOGGED IN
        session_start();
        $_SESSION['username']=$rowUser['username'];
        $_SESSION['user_id']=$rowUser['user_id'];
        $_SESSION['login_time']=date(" H:i:s d-m-Y");
 
        header("Location: main.php");
?>

Open in new window

0
paul_taylor_22
Asked:
paul_taylor_22
1 Solution
 
Roger BaklundCommented:
You are not escaping the input. Change this:

$username=$_POST['username'];

...into this:

$username=$_POST['username'];
if(get_magic_quotes_gpc())
  $username=stripslashes($username);
$username=mysql_real_escape_string($username);

Make a function for this, this code should be excecuted for all user input that goes into a sql statement.

You have separate messages for bad username and bad password. Even if the message is the same, the extra statements executed for bad passwords might allow the hacker to identify valid user names by measuring the response time. I suggest you execute something like "usleep(rand(100000,200000));" for all failed logins to camouflage the fact that you update the user record in the db when the correct username is entered.
0
 
paul_taylor_22Author Commented:
OK, thanks for that, its in place and appears to work.
0

Featured Post

Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Tackle projects and never again get stuck behind a technical roadblock.
Join Now