0

I am trying to create a login system. Below are the examples of my code.

index.php

<?php
session_start();
?>
<!doctype html>
<html lang="en">
<head>
    <title>Login System</title>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
    <link rel="stylesheet" href="vendors/bootstrap/css/bootstrap.min.css">
    <script src="vendors/jquery/jquery.min.js"></script>
    <script src="vendors/bootstrap/js/bootstrap.bundle.min.js"></script>
</head>
<body>
<div class="container-fluid">
<h2 class="text-center">Login System</h2>
    <div class="card">
        <div class="card-header">
            <h4>Sign In</h4>
        </div>
        <div class="card-body">
            <form role="form" action="loginform.php" method="post">
                <div class="form-group">
                    <label for="inputusername">Username:</label>
                    <input class="form-control" type="text" id="inputusername" name="username" required>
                </div>
                <div class="form-group">
                    <label for="inputpassword">Password:</label>
                    <input class="form-control" type="text" id="inputpassword" name="password" required>
                </div>
                <?php
                if (isset($_SESSION['error'])) {
                    echo '<div class="alert alert-danger text-center" role="alert">' . $_SESSION['error'] . '</div>';
                }
                ?>
                <button type="submit" class="btn btn-primary btn-block" name="login">Sign In</button>
            </form>
        </div>
    </div>
</div>
</body>
</html>
<?php
unset($_SESSION['error']);
?>

loginform.php

<?php
session_start();

include 'dbconfig.php';

if (isset($_POST['login'])) {
    $username = $_POST['username'];
    $password = sha1($_POST['password']);

    $login_query = "SELECT user_name, user_pass, user_role FROM users WHERE user_name = ?";

    $login_stmt = mysqli_prepare($dbcon, $login_query);

    mysqli_stmt_bind_param($login_stmt, "s", $username);

    mysqli_stmt_execute($login_stmt);

    mysqli_stmt_bind_result($login_stmt, $user_name, $user_pass, $user_role);

    if (mysqli_stmt_fetch($login_stmt) == true) {
        if ($user_pass == $password) {
            session_regenerate_id();
            $_SESSION['logged_in'] = true;
            $_SESSION['username'] = $user_name;
            $_SESSION['userrole'] = $user_role;
            header('Location: dashboard.php');
        }
        else {
            $_SESSION['error'] = "You have entered incorrect password.";
            header('Location: index.php');
        }
    }
    else {
        $_SESSION['error'] = "You have entered incorrect username.";
        header('Location: index.php');
    }
}

Is my code good enough to prevent SQL injection? If not, what can be done to improve?

P.S: As I have understood from resources found in the Internet that parameterized query is enough to prevent SQL injection. I might be wrong. If so, please explain.

Thanks.

Nirjhar
  • 9
  • 6
  • 3
    Yes this example should be secure against SQL injection. Write some tests, if you want to prove it. On another security-related issue though, sha1 is out of date. Don't do your own hashing. Learn about PHP's built-in, up-to-date, secure [password hashing](https://www.php.net/manual/en/faq.passwords.php) functionality instead. – ADyson Feb 10 '21 at 11:44
  • Also a HTML password field should use `type="password"` so the password isn't visible when typing it. You must have seen this in most websites. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/password for more details and options. – ADyson Feb 10 '21 at 11:45
  • 1
    @AlivetoDie you should never store plain text passwords, as already mentioned you should be using password hash/verify. – Nigel Ren Feb 10 '21 at 11:55
  • Thanks everyone. stickybit - I had seen that example. That's what I mentioned in the footnote. :) ADyson - Yeah, that was a typo from my end, but thanks for pointing it out. I will be more careful from next time. AlivetoDie - Could you please explain why checking password against username is necessary? And yes, I have read all the comments. :) – Nirjhar Feb 10 '21 at 12:01
  • @Nirjhar sorry. I have misunderstood things. go with what ADyson and Nigel Ren said. Apology from my side. – Alive to die - Anant Feb 10 '21 at 12:12
  • My only concern is incase `username` are not unique.(I hope they are) – Alive to die - Anant Feb 10 '21 at 12:14
  • Yeah, the usernames should be unique. I will later implement a checking for that. – Nirjhar Feb 10 '21 at 12:31

0 Answers0