PHP Login and Registration System Security

Question

I've followed many tutorials to make my user system as secure as possible. I don't know what else I might've missed. The website will be released to hundreds of thousands of users. I am clueless to what security flaw I missed and I'll appreciate it if any of you advanced users out there can give me feedback on how to improve.

Registration

function validateDate($date, $format = 'Y-m-d H:i:s'){
$d = DateTime::createFromFormat($format, $date);
return $d && $d->format($format) == $date;
    }

    if (isset($_POST['register'])) {
if (empty($_POST['date'])) {
    $error = '1';
    $dateError = "Please enter a birthday date.";
} else {
    if ((validateDate($_POST['date'], 'm/d/Y')) == FALSE) {
        $error = '1';
        $dateError = "Please enter a valid birthday date.";
    }
}

if (empty($_POST['username'])) {
    $error = '1';
    $usernameError = "Please enter an username.";
} else
if (strlen($_POST['username']) < 3) {
    $error = '1';
    $usernameError = "Username must have at least 3 characters.";
} else
if (!preg_match("/^[a-z0-9\d_]{3,16}$/i", $_POST['username'])) {
    $error = '1';
    $usernameError = "Username can only contain alphabets, underscore characters and numbers.";
} else {
    $sql = "SELECT * FROM users WHERE username=?";
    $stmt = $mysql->prepare($sql);
    $stmt->bind_param('s', $_POST['username']);
    $stmt->execute();
    $result = $stmt->get_result();
    $results = $result->num_rows;
    if ($results >= 1) {
        $error = '1';
        $usernameError = "Username is already in use.";
    }
}

if (!empty($_POST["name"])) {
    if (!preg_match("^(\s)*[A-Za-z]+((\s)?((\'|\-|\.)?([A-Za-z])+))*(\s)*$^", $_POST['name'])) {
        $error = '1';
        $nameError = "Name can only contain alphabets and spaces.";
    }
}

if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
    $error = '1';
    $emailError = "Please enter a valid email address.";
} else {
    $xsql = "SELECT * FROM users WHERE email=?";
    $xstmt = $mysql->prepare($xsql);
    $xstmt->bind_param('s', $_POST['email']);
    $xstmt->execute();
    $xresult = $xstmt->get_result();
    $xresults = $xresult->num_rows;
    if ($xresults >= 1) {
        $error = '1';
        $emailError = "Email is already in use.";
    }
}

if (empty($_POST['password'])) {
    $error = '1';
    $passError = "Please enter a password.";
}
else {
    if (strlen($_POST['password']) < 6) {
        $error = '1';
        $passError = "Password must have at least 6 characters.";
    }
}

if ($error == '0') {
    $name = $_POST['name'];
    $name = substr($name, 0, 40);
    $username = $_POST['username'];
    $username = substr($username, 0, 16);
    $date = $_POST['date'];
    $email = $_POST['email'];
    $password = $_POST['password'];
    $trn_date = date("Y-m-d H:i:s");
    $ip = '';
    if (getenv('HTTP_CLIENT_IP')) {
        $ip = getenv('HTTP_CLIENT_IP');
    }
    else
    if (getenv('HTTP_X_FORWARDED_FOR')) {
        $ip = getenv('HTTP_X_FORWARDED_FOR');
    }
    else
    if (getenv('HTTP_X_FORWARDED')) {
        $ip = getenv('HTTP_X_FORWARDED');
    }
    else
    if (getenv('HTTP_FORWARDED_FOR')) {
        $ip = getenv('HTTP_FORWARDED_FOR');
    }
    else
    if (getenv('HTTP_FORWARDED')) {
        $ip = getenv('HTTP_FORWARDED');
    }
    else
    if (getenv('REMOTE_ADDR')) {
        $ip = getenv('REMOTE_ADDR');
    }
    else {
        $ip = '127.0.0.1';
    }

    $activation = md5($email . time());
    $sql = "INSERT into `users` (username, name, password, email, birthday, status, activation, ip, trn_date, theme, picture, views, currency, publicprofile, showaboutyourself, showviews, changedecimal, sortby, apiscrape, badge, sortfeed, who_can_pm_me, who_can_notify_me, google_auth_code) VALUES (?, ?, ?, ?, ?, ?, '0', ?, ?, ?, 'light', 'default.png', '1', 'USD', '1', '1', '1', '0', 'latestadded', 'coinmarketcap', 'USER', 'relevance', 'all', 'all', ?)";
    $options = ['cost' => 15, ];
    $pdw = password_hash($password, PASSWORD_BCRYPT, $options);
    $stmt = $mysql->prepare($sql);
    $stmt->bind_param('sssssssss', $username, $name, $pdw, $email, $date, $activation, $ip, $trn_date, $secret);
    $stmt->execute();



    // log them in after registration
    $sql = "SELECT id FROM `users` WHERE email=?";
    $stmt = $mysql->prepare($sql);
    $stmt->bind_param('s', $email);
    $stmt->execute();
    $result = $stmt->get_result();
    $row = $result->fetch_assoc();
    $userid = $row["id"];

    $usersession = substr("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", mt_rand(0, 50) , 1) . substr(md5(time()) , 1) . bin2hex(openssl_random_pseudo_bytes(25));
    $now = time();
    $expires = time() + 86400;
    date_default_timezone_set('Europe/Amsterdam');
    $date = date('d-m-Y');
    $sql = "INSERT INTO sessions (SESSION_ID, SESSION_USER_ID, SESSION_IP, SESSION_EXPIRES, SESSION_DATE, SESSION_ACTIVE, SESSION_AGENT) VALUES (?,?,?,?,?,'1',?)";
    $stmt = $mysql->prepare($sql);
    $stmt->bind_param('ssssss', $usersession, $userid, $ip, $expires, $date, $_SERVER['HTTP_USER_AGENT']);
    $stmt->execute();
    setcookie("MySite_Session_ID", $usersession, $expires, '/');
    header("Location: /");
    die();
}

Login

if (isset($_POST['login'])) {
usleep(500000); // Slow down brute-forcers
$error = '0';
$email = $_POST['email'];
$password = $_POST['password'];
if (empty($email)) {
    $error = '1';
    $userError = "Please enter your email.";
}

if (empty($password)) {
    $error = '1';
    $passError = "Please enter your password.";
}

if ($error == '0') {
    $sql = "SELECT id,password,status FROM `users` WHERE email=?";
    $stmt = $mysql->prepare($sql);
    $stmt->bind_param('s', $email);
    $stmt->execute();
    $result = $stmt->get_result();
    $results = $result->num_rows;
    if ($results >= 1) {
        $row = $result->fetch_assoc();
        if ($row['status'] != 0) {
            if (password_verify($password, $row["password"])) {
                // Password success, log them in

                $userid = $row["id"];
                $useragent = $_SERVER['HTTP_USER_AGENT'];
                $usersession = substr("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", mt_rand(0, 50) , 1) . substr(md5(time()) , 1) . bin2hex(openssl_random_pseudo_bytes(25));
                $now = time();
                $expires = time() + 86400;
                if (isset($_POST['rememberme'])) {
                    $expires = time() + 15768000;
                }
                date_default_timezone_set('Europe/Amsterdam');
                $date = date('d-m-Y');
                $sql = "INSERT INTO sessions (SESSION_ID, SESSION_USER_ID, SESSION_IP, SESSION_EXPIRES, SESSION_DATE, SESSION_ACTIVE, SESSION_AGENT) VALUES (?,?,?,?,?,'1',?)";
                $stmt = $mysql->prepare($sql);
                $stmt->bind_param('ssssss', $usersession, $userid, $_SERVER['REMOTE_ADDR'], $expires, $date, $_SERVER['HTTP_USER_AGENT']);
                $stmt->execute();
                setcookie("MySite_Session_ID", $usersession, $expires, '/');
                header("Location: /");
            } else {
                $passError = "Invalid password.";
            }
        } else {
            $userError = 'Your account has not been activated. Please check your email.';
        }
    }
}
}

Logout

$mysql = mysqli_connect('localhost', 'root', '');
mysqli_select_db($mysql,"mysite");

$sql = "DELETE FROM sessions WHERE SESSION_ID=?";
$stmt = $mysql->prepare($sql);
$stmt->bind_param('s', $_COOKIE['MySite_Session_ID']);
$stmt->execute();

setcookie ('MySite_Session_ID', "", 1);
setcookie ('MySite_Session_ID', false);
unset($_COOKIE['MySite_Session_ID']);

header("Location: /");
exit();

Check if they're logged in

ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);
$mysql = mysqli_connect('localhost', 'root', '');
mysqli_select_db($mysql, "mysite");
date_default_timezone_set('Europe/Amsterdam');

if (isset($_COOKIE['MySite_Session_ID'])) {
$usersession = $_COOKIE['MySite_Session_ID'];
$sql = "SELECT SESSION_USER_ID,SESSION_EXPIRES FROM sessions WHERE SESSION_ID = ? AND SESSION_ACTIVE = '1' AND SESSION_AGENT = ?";
$stmt = $mysql->prepare($sql);
$stmt->bind_param('ss', $usersession, $_SERVER['HTTP_USER_AGENT']);
$stmt->execute();
$dasdasresult = $stmt->get_result();
$results = $dasdasresult->num_rows;
if ($results >= 1) {
    // Is logged in

    $row = $dasdasresult->fetch_assoc();
    $userid = $row["SESSION_USER_ID"];
    $expiretime = $row["SESSION_EXPIRES"];

    if ($expiretime > time()) {
        // Session did not expire

        $sql = "SELECT * from users WHERE id=?";
        $stmt = $mysql->prepare($sql);
        $stmt->bind_param('s', $userid);
        $stmt->execute();
        $dasdasresultx = $stmt->get_result();
        $results = $dasdasresultx->num_rows;
        if ($results >= 1) {
            // User exists

            $row = $dasdasresultx->fetch_assoc();

            $username                       = $row["username"];
            $name                           = $row["name"];
            $userid                         = $row["id"];
            $email                          = $row["email"];
            $loggedin                       = 1;
        } else {
            // User does not exist

            header("Location: /login");
            exit();
        }
    } else {
        // Session expired

        $sql = "UPDATE sessions SET SESSION_ACTIVE = '0' WHERE SESSION_ID = ?";
        $stmt = $mysql->prepare($sql);
        $stmt->bind_param('s', $usersession);
        $stmt->execute();

        header("Location: /login");
        exit();
    }
} else {
    header("Location: /login");
    exit();
}
} else {
    header("Location: /login");
    exit();
}

Answer 1

Security

• make sure your site is running over HTTPS.
• add httponly parameter to setcookie()
• you'd like to check whether openssl_random_pseudo_bytes()'s result was cryprographically strong, using the function's 2nd parameter

Some random nitpicks

$usersession = substr("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", mt_rand(0, 50) , 1) . substr(md5(time()) , 1)

This code is rather silly and could be safely discarded.

 "Name can only contain alphabets and spaces.";

Suppose your site won't be overly popular in Ireland.
Yours, Patrick O'Neal

if (getenv('HTTP_CLIENT_IP')) {

Apart from the fact that registration IP is rather useless as is, the code is one of old PHP users' superstitions and should never be used. Any IP related stuff should be taken from REMOTE_ADDR only, not from various HTTP headers.

if (empty($email)) {

It makes no sense to use empty() for the previously defined variable. if (!$email) { would be enough.

 $sql = "SELECT id FROM `users` WHERE email=?";

mysqli_insert_id()

ini_set('display_errors', 1);

Should be 0 on a live site

"UPDATE sessions SET SESSION_ACTIVE = '0' WHERE SESSION_ID = ?";

I don't see much point in the SESSION_ACTIVE field, as it's just duplicating the timeout

setcookie ('MySite_Session_ID', "", 1);
setcookie ('MySite_Session_ID', false);

There should be only one call and parameter mach ones with setting a session cookie, otherwise it won't be removed

unset($_COOKIE['MySite_Session_ID']);

this call is useless at the end of the script

$xsql = "SELECT * FROM users WHERE email=?";
$xstmt = $mysql->prepare($xsql);
$xstmt->bind_param('s', $_POST['email']);
$xstmt->execute();
$xresult = $xstmt->get_result();
$xresults = $xresult->num_rows;

Did you notice that you are repeating this kind of code all the time? There is one programming concept which is often underestimated by PHP users. It is called user-defined functions. What about calling a code like this:

$sql = "SELECT count(*) FROM users WHERE email=?";
$xresults = getSQLResult($mysql, $sql, $_POST['email']);

Answer 2

All code must be formatted and indented properly, otherwise you scare off potential readers. There are enough other things to worry about when reviewing code, so don't distract your readers.

So instead of this:

function abc() {
    if (condition) {
if (condition2) {
    action();
}
        }
    }

The code must look like:

function abc() {
    if (condition) {
        if (condition2) {
            action();
        }
    }
}

So when you say you beautified your code, did you actually do it, or did your IDE do it? It should always be the IDE for basic things like indentation and spacing. The first few lines of your code look very inconsistent in this regard.