PHP & MySQLi Login Script - Is it safe from Injection?

Question

I've been geeking up on MySQLi prepared statements to avoid potential SQL injection and I have managed to modify my legacy code (previously just using mysqli_real_escape_string).

I am looking for some advice regarding if this code template is safe enough to use as I will be looking to update the rest of my code across my site. I received some help from Fred-ii so thanks for that.

One issue I know is present is the use of $_SERVER['HTTP_REFERER'] but I will be creating a workaround via an array of acceptable URLs.

<?php 
// Starting Session

session_start(); 
include '../connection.php';

// Previous page variable

$previous = $_SERVER['HTTP_REFERER'];

//Acquire POST values Please note: Sanity checks will be added for these =)

$myusername = $_POST["username"];
$mypassword = $_POST["password"];

// Preparing the statement

$stmt = $connection-> prepare("SELECT username, password FROM $tbl_name WHERE username= ? ");
$stmt -> bind_param("s", $myusername);

// Sanity checks on the statement and binding

if($stmt ->execute() == true) {
    $stmt -> bind_result($user, $pass);
    $stmt -> fetch();

    //Checking hashed password

    if (password_verify($mypassword, $pass)) {

        $_SESSION['login_user']= $myusername;   
        header("location: ../member"); // Redirecting To Other Page         

    } else {

        $_SESSION["login_error"] = "<div id='loginAlert'> <p style='font-size: 1em; padding: 5px; text-align: center; background: #f2f2f2; 
        border: 1px solid #f2f2f2'>Incorrect username or password, please try again.</p> </div>";

        header('Location:'.$previous);

    }


    }

/* Close statement */
$connection-> close(); 

Looking for any feedback to improve this and any other recommendations.

Thanks. Stan.

Answer 1

There's no need for the $user variable (it's already in $myusername) so don't query for it either.

Please, deal with the $previous = $_SERVER['HTTP_REFERER']; now. You don't want to allow a malicious user to edit the HTTP headers you send.

What do you plan to do if any of the database calls fail? You don't seem to be checking for nor handling that. Also, it's best to have the exact same behavior on "invalid username" as well as "password supplied does not match". I'm not sure if your code does that or not.

I didn't think people usually put spaces around the -> method call operator, but at least be consistent with your spacing.