HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Is this login page safe and can it be optimized?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thiscanloginoptimizedpagesafeand

Problem

 prepare('SELECT id FROM user WHERE username = ? AND password = ?');
    $stmt -> bind_param('ss', $username, $password_hash);
    $stmt -> execute();
    $stmt -> bind_result($id);
    $stmt -> fetch();

    if($id){
        $_SESSION['username'] = $username;
        $_SESSION['email'] = $email;
        header('location: control-painel.php');
        die();
    }
    else{
        $login_error = 'User or Password invalid';
    }        
}
}
?>

       
    
    
    
    

    
        
            Username: 
            
            Password: 
            
            
        
    

Solution

The first thing I spotted was this: control-painel.php. I'm not sure if that's an error with your page name or code, but whichever it is needs to be fixed.


Can it be optimized?

The best optimisation is only doing what's necessary. When checking the login, you don't check whether they've even been specified until after you hash it, consider reordering it like so:

$username = $_POST['username'];
$password = $_POST['password'];
if(empty($username))
{
    $user_error = 'Please insert a username';
}
if(empty($password))
{
    $pass_error = 'Please insert a password';
}
elseif(!$user_error) //Password can't be empty here, don't need to check again
{
    $cost = '11';
    $salt = 'Cf1f11ePArKlBJomM0F6aJ';
    $password_hash = crypt($password, '$2a

So the hashing is only performed when you have a username and password. . $cost . '

So the hashing is only performed when you have a username and password. . $salt . '

So the hashing is only performed when you have a username and password.); $id = 0; $stmt = $mysqli->prepare('SELECT id FROM user WHERE username = ? AND password = ?'); $stmt->bind_param('ss', $username, $password_hash); $stmt->execute(); $stmt->bind_result($id); $stmt->fetch(); if($id) { $_SESSION['username'] = $username; $_SESSION['email'] = $email; header('location: control-painel.php'); die(); } else { $login_error = 'User or Password invalid'; } }


So the hashing is only performed when you have a username and password.

Code Snippets

$username = $_POST['username'];
$password = $_POST['password'];
if(empty($username))
{
    $user_error = 'Please insert a username';
}
if(empty($password))
{
    $pass_error = 'Please insert a password';
}
elseif(!$user_error) //Password can't be empty here, don't need to check again
{
    $cost = '11';
    $salt = 'Cf1f11ePArKlBJomM0F6aJ';
    $password_hash = crypt($password, '$2a$' . $cost . '$' . $salt . '$');
    $id = 0;
    $stmt = $mysqli->prepare('SELECT id FROM user WHERE username = ? AND password = ?');
    $stmt->bind_param('ss', $username, $password_hash);
    $stmt->execute();
    $stmt->bind_result($id);
    $stmt->fetch();
    if($id)
    {
        $_SESSION['username'] = $username;
        $_SESSION['email'] = $email;
        header('location: control-painel.php');
        die();
    }
    else
    {
        $login_error = 'User or Password invalid';
    }
}

Context

StackExchange Code Review Q#51937, answer score: 3

Revisions (0)

No revisions yet.