User Data Security

Question

I'm working on my site, and need help knowing what I need to do to improve the security of my login system.

If you feel so inclined, I would love some help to tell me what the exact changes that I need to make, but if you could just tell me what other security measures I need to add, that would be amazing.

This is a login system.

Specifically, I need help using MySQLi prepared statements.

I'm very confused about that, but any other things I should add to secure user data would be great.

Register.php

//Get all POST data from user trying to register
$username = $_POST['username'];
$password =  $_POST['password'];
$name = $_POST['name'];
$email = $_POST['email'];
$lat = $_POST['lat'];
$lon = $_POST['lon'];

//Choose wether the user will be "it" or "not it"
$choose_it = array('it', 'not it');
$key = array_rand($choose_it);
$it = $choose_it[$key];

$message = "";

$allEmails = "";
$allUnames = "";

function strong_crypt($input, $rounds = 81)
{
    $salt = "";
    $salt_chars = array_merge(range('A','Z'), range('a','z'), range(0,9));

    for($i=0; $i < 22; $i++)
    {
        $salt .= $salt_chars[array_rand($salt_chars)];
    }
    return crypt($input, sprintf('$2a$%02d$', $rounds) . $salt);
}

//Get emails from registered users and check if it has already been used
$AllPeople = "SELECT email, uname FROM login WHERE email = '$email'";
$query = mysqli_query($conn,$AllPeople);

while($row = mysqli_fetch_array($query))
{
    $allEmails = $row['email'];
    $allUnames = $row['uname'];
}

if($allEmails != "")
{
    $message = "That email already exists. You can only have one account per email address.";
}

if($allUnames != "")
{
    $message = "That username has already been taken. Please choose a different one.";
}

if($allEmails === "" && $allUnames === "")
{
    $hash = strong_crypt($password);

    mysqli_query($conn, "INSERT INTO login (name, uname, pword, email, lat, lon, it) VALUES ('$name', '$username', '$hash', '$email', '$lat', '$lon', '$it')");

    $message = "goodtogo";
}

echo json_encode(array("message" => $message, "name" => $name, "it" => $it));

mysqli_close($conn);

Login.php

$username = $_POST['username'];
$inPword = $_POST['password'];
$lat =  $_POST['lat'];
$lon =  $_POST['lon'];

$returnmessage = "";
$loggedin = "";

$password_hash = "";
$loggedinstatus = "";

$sel = mysqli_query($conn,"SELECT pword, loggedin FROM login WHERE uname = '$username'");

if(mysqli_num_rows($sel) < 1)
{
    $returnmessage = "BadLogin";
}

while($fetch = mysqli_fetch_array($sel,MYSQLI_ASSOC))
{
    $password_hash = $fetch['pword'];
    $loggedinstatus = $fetch['loggedin'];
}

if(crypt($inPword, $password_hash) == $password_hash)
{
    switch($loggedinstatus)
    {
        case  "no":
            $returnmessage = "AllGood";
            $sel = mysqli_query($conn,"UPDATE login SET loggedin='yes' WHERE uname = '$username'");
            break;
        defaut:
            $returnmessage = "alreadyin";
            break;
    }
}
else
{
    $returnmessage = "error";
}
echo json_encode(array("message" => $returnmessage, "uname" => $username));

mysqli_close($conn);

Answer 1

The first thing I will tell you to do is to look into SQL injections. Your scripts are providing a pretty easy way of potentially exposing security holes. using mysqli_real_escape_string to escape any data going into your database is one of the better sanity checks.

In your registration, you want to check that the specified email or username does not exist within the login table. You are not currently checking for the username - only the email.

Personally, I would split them email and username data off, gathered from the query, into separate arrays, then perform a count on the array to ascertain whether the information exists or not - others may do this differently.

With your login script, again, parameters need to be escaped before being placed in the SQL query.

When comparing the number of available login users we have, you are doing: mysqli_num_rows($sel) < 1, it might be prudent to check that we have 0 - since we aren't going to have anything less than 0: 0 == mysqli_num_rows($sel).

I would advise adding a LIMIT 1 to the SQL, simply because, if for some reason we end up with two users from the search query, we are going to be overwriting the variables used to test the supplied credentials.