-1

I am building a website with a user page. Currently it is set up in the following way:

The user types their username and password into corresponding fields then the user pressed submit.

The js code sends a get request to a php script with the password and username in plain text.

The php returns a randomly generated session token to the js code if the credentials are correct and an error message otherwise.

All future interactions the user engages in while logged on will now require the session token to be provided in any get requests. The php will then check that the session token is correct.

In the back end, the user name is stored in a database along side a salt and the hash (sha256) of the salted password. (the salt is randomly generated upon account creation).

My question is as follows: Is there anything in the description above that seems insecure? If so, what should be done instead. More broadly, what are the best practices around setting up a user login page or account system for a website. Thanks

Jodast
  • 1,279
  • 2
  • 18
  • 33
Mathew
  • 1,116
  • 5
  • 27
  • 59

1 Answers1

1

Why are you trying to re-invent the wheel? PHP already has built-in password encryption functions so why using Sha256 + Salt?

Again they are two type of authentication:

  1. Session Based Login
  2. Token Based Login.

From your write-up you are combining session login with token login. You will need to decide which one that you want to apply.

Consequently they are a lot of PHP validation or sanitization functions that you need to know to keep your code more secured.

1.) use strip_tags()

This will strips out all html elements from form inputs or variables Eg

$email = strip_tags($_POST['email']);

2.) use htmlentities or htmlspecialchars to prevent XSS Attack.

This converts HTML tags to their respective entities. It's only used when printing or echoing result to HTML page to. You will see how I used it in the welcome.php page.

See applications:

$email = htmlentities($_POST['email']);

3.) escaping variables against SQL injection Attack

If you are using Mysqli, the best SQL method to be used is prepared Statement. Alternatively, you can still escape variables using mysqli_real_escape_string() functions.

See application:

// escape variables Against sql injections
$email = mysqli_real_escape_string($conn, $_POST['email']);

4.) If you are using session based login, You need to use sessionId regenerate method. This will help to regenerate new session Id as user login thus preventing session fixation attack. Do not worry you will need how to use it in the login.php code below.

See application:

// first you will need to initialize sessions
session_start();
session_regenerate_id();

This are just few among other security measures.

Let's have a look at Session based login using PHP password verify functions

Assume this is your registration.php:

<?php 
$conn = mysqli_connect("localhost","root","","demo");
 
if(!$conn){
    die("Connection error: " . mysqli_connect_error()); 
}

if(isset($_POST['submit'])){
        $firstName = mysqli_real_escape_string($conn,$_POST['first_name']);
        $surName = mysqli_real_escape_string($conn,$_POST['surname']);
        $email  = mysqli_real_escape_string($conn,$_POST['email']);
        $password = mysqli_real_escape_string($conn,$_POST['password']);
        
        $options = array("cost"=>4);
        $hashPassword = password_hash($password,PASSWORD_BCRYPT,$options);
        
        $sql = "insert into users (first_name, last_name,email, password) value('".$firstName."', '".$surName."', '".$email."','".$hashPassword."')";
        $result = mysqli_query($conn, $sql);
        if($result)
        {
            echo "Registration successfully";
        }
    }
?>

This is now how your login.php code will look like:

<?php 
$conn = mysqli_connect("localhost","root","","demo");
 
if(!$conn){
    die("Connection error: " . mysqli_connect_error()); 
}
if(isset($_POST['submit'])){
    $email = mysqli_real_escape_string($conn,$_POST['email']);
    $password = mysqli_real_escape_string($conn,$_POST['password']);
    
    $sql = "select * from users where email = '".$email."'";
    $rs = mysqli_query($conn,$sql);
    $numRows = mysqli_num_rows($rs);
    
    if($numRows  == 1){
        $row = mysqli_fetch_assoc($rs);
        if(password_verify($password,$row['password'])){
            echo "Password verified and ok";

// initialize session if things where ok.


session_start();
session_regenerate_id();

$_SESSION['surname'] = $row['surname'];
$_SESSION['first_name'] = $row['first_name'];
$_SESSION['email'] = $row['email'];

// take me to welcome.php page
header('Location: welcome.php');

        }
        else{
            echo "Wrong Password details";
        }
    }
    else{
        echo "User does not exist";
    }
}

?>

Welcome.php will now look like code below to show authenticated users session info. Use htmlentities or htmlspecialchars to prevent XSS Attack.

<?php echo htmlentities($_SESSION['surname'], ENT_QUOTES, "UTF-8"); ?>

<?php echo htmlspecialchars($_SESSION['first_name'], ENT_QUOTES, "UTF-8"); ?>
<?php echo htmlentities($_SESSION['email'], ENT_QUOTES, "UTF-8"); ?>

Now in your post I saw where you wrote sending a generated token with every http request. In this case I guess you are trying to mitigate CSRF Attack.

Here is the best and most secure way to do once you're logged in.

To prevent CSRF you'll want to validate a one-time token, POST'ed and associated with the current session. Something like the following.

On the page where the user requests eg to insert a record for payments:

payment.php

<?php
 session_start();
 $token= md5(uniqid());
 $_SESSION['payment_token']= $token;
 session_write_close();
?>
<html>
<body>
<form method="post" action="payment_save.php">
 <input type="hidden" name="token" value="<?php echo $token; ?>" />
Amount: <input type="hidden" name="token" value="100 usd" />
<input type="submit" value="submit" />

</form>
</body>
</html>

Then when it comes to actually inserting the record:

payment_save.php

<?php
 session_start();
 $token = $_SESSION['payment_token'];
 unset($_SESSION['payment_token']);
 session_write_close();
 if ($token && $_POST['token']==$token) {
   // Insert the record for payment
 } else {
   // log message. You are vulnerable to CSRF attack.
 }
?>

The token should be hard to guess, unique for each insert request, accepted via $_POST only and expire after a few minutes (expiration not shown in this illustration).

halfer
  • 19,824
  • 17
  • 99
  • 186
Nancy Moore
  • 2,322
  • 2
  • 21
  • 38
  • 2
    Why even mention escaping SQL strings in a best-practice / security related post, let alone provide an example of it (and no prepared statement example)? – Phil Mar 08 '19 at 20:56
  • Please do not suggest `mysqli_real_escape_string` as a prevention for SQL injection. I know it might work if used properly, but we have native prepared statements which are much better. `mysqli_real_escape_string` can easily be misused. Even your example code is misusing it with `$password = mysqli_real_escape_string($conn,$_POST['password']);`. You then proceed to hash the password, which will not only change its value, but also be completely ineffective for SQL injection prevention. – Dharman Oct 08 '19 at 22:49
  • It is a very bad idea to use `die(mysqli_connect_error());` in your code, because it could potentially leak sensitive information. See this post for more explanation: [mysqli or die, does it have to die?](https://stackoverflow.com/a/15320411/1839439) – Dharman Oct 08 '19 at 22:50
  • 1
    Thanks @Dharman it was an answer that I gave when i was still learning. am prepared statement fan. I barely use mysqli. I use PDO for all db connections – Nancy Moore Oct 08 '19 at 22:54
  • When should I use `session_regenerate_id()`? You do not seem to explain why or when it should be used. If you are making this a comprehensive guide why not mention other vulnerabilities related to sessions? See https://paragonie.com/blog/2015/04/fast-track-safe-and-secure-php-sessions – Dharman Oct 08 '19 at 22:55
  • something like session hijacking which can be mitigated with https – Nancy Moore Oct 08 '19 at 22:56
  • `session_regenerate_id()` for mitigation of session fixation attack – Nancy Moore Oct 08 '19 at 22:57
  • 1
    It is an interesting answer, but in my opinion you have mentioned too many topics (the question was too broad itself) and in the result you skimmed a lot of important details. The question didn't mention mysqli from what I can see, so it might be a good idea to propose PDO. If you decided to use mysqli, please at least suggest OOP. – Dharman Oct 08 '19 at 22:58
  • okay i will keep that in mind. Thanks – Nancy Moore Oct 08 '19 at 22:59
  • And lastly, maybe a personal opinion, but I never saw a use for `strip_tags()` and I consider it a useless function. Your answer mentions it, but doesn't explain why it should be used. Why wouldn't I want HTML tags in my values? – Dharman Oct 08 '19 at 23:00