I am working on a login, to privatize a web application that I am doing. My knowledge is not that basic, but not the best and I need help.
There is no registration method. The registration will be done by me personally from the database. What I need is advice or ways to make this login and later session controls, more secure, more reliable, a better structure of the .php documents in the root directory...
At the moment, the only thing I am applying is PDO and the sentences prepare
to avoid SQL injections.
The structure of the documents are:
login.php (Here is the form, and where the functions are executed). This is the PHP code before the header to manage the login:
if (isset($_POST['entrar'])) { require "functions.php"; $error = []; $user = ValidUser($error); if(empty($error)){ logearUsuario($user); header("Location: application.php"); } }
The names of the variables to be collected by the form are: user(user), pass(password) and enter(submit)
conn.php (In this document is what is necessary to connect to the database, in this case MySQL, but in the future it will change to PostgreSQL. It is a function that includes everything necessary to connect)
function connection(){ try{ $dsn = 'mysql:host=localhost; dbname=controlusuarios'; $userBBDD = 'root'; $passBBDD = ''; $conn = new PDO($dsn, $userBBDD, $passBBDD); return $conn; }catch(Exception $e){ die('Error: ' . $e->GetMessage()); } }
functions.php (Contains several functions. One to validate the user, and another to log in. The login condition occurs in the login.php document)
function validUser(&$error){ if((!isset($_POST['user'])) || (!isset($_POST['pass']))){ $error[0] = "Usuario y/o contraseña incorrectos."; return null; } $user = $_POST['user']; $pass = $_POST['pass']; if(($user == '') || ($pass == '')){ $error[0] = "Usuario y/o contraseña incorrectos."; return null; }else{ $con = connection(); $sql = "SELECT name FROM usuarios WHERE name = :user AND password = :pass"; $query = $con->prepare($sql); $query-> bindParam(':user', $user); $query-> bindParam(':pass', $pass); $query-> execute(); $contador = $query -> rowCount(); if($contador != 1){ $error[0] = "Usuario y/o contraseña incorrectos."; return null; } $con = null; return $user; } } function logearUsuario($user){ session_start(); $_SESSION['name'] = $user; }
I'm not a security expert, but based on the code you've shared, I see some issues/points that could be improved:
The session id is not changed after a state change in the login
It really doesn't show much how you treat the session, but at first glance it seems that you open it when the user logs in (I don't know if it opens before or not) and save the user's name, without seeming to do anything else.
Regenerating the session id each time there is a change in a user's privilege level (eg when logging in or logging out) is a recommended practice by OWASP that serves to prevent multiple attacks such as session hijacking , session or session override (known by its acronym in English CSRF or XSRF).
In PHP you can regenerate the session id using
session_regenerate_id()
, which applied to your code would look something like this:The session does not seem to expire
Again, you don't really see much of how you treat the session, but when you log in it seems to only save the username, nothing else.
It would be ideal for you to save the login date and then, every time an operation is performed or a page is loaded, check the date and verify that it is within an appropriate range to perform an appropriate action (refresh or log out).
For example, if more than 10 minutes have passed without action, expire the session (regenerating the id with what was stated in the previous point) and send the user to login. And if no more than those 10 minutes have passed, then update the counter again to give it 10 more minutes.
As stated on the OWASP page on session expiration :
Provisions must be made to close the session when it expires on both the client and server side (although we only see the PHP part here).
You do not penalize mistakes
When a user enters their password wrong, you simply show them an error message and that's it. What happens if the user has not made a mistake but is an attacker trying to see if there is luck and guesses your password?
By not penalizing login failures, the code is vulnerable to brute force attacks . An attacker (which could be a program/bot) only has to send many password attempts until one works and gains access to the privileged account.
A solution to brute force login attacks would be to add some kind of lockout after several attempts. For example: after X failures, lock the account for M minutes. And if after that time has elapsed it fails again, increase the value of M following some pattern of severity.
Another possible solution would be to require a CAPTCHA after X failed attempts and each time it fails again.
The database connection information is in the code itself
As indicated on this OWASP page and also on this other one (my translation):
Also, if for any reason the server stops processing the PHP code correctly and renders it as plain text (rare, but it could happen), the connection data will be visible to anyone.
Ideally, the database connection information will be in a separate file outside the root of the website. That way it will not be accessible from outside the server, and will not be available to possible snoopers (external or internal).
The keys are not hashed
This is a problem if someone manages to break into your system and access the database: login systems must be designed taking into account the possibility that at some point they can fail and the data ends up being compromised ( as mentioned in OWASP ) .
In that sense, it is essential to protect the user's account and having the passwords in plain text in the database is a bad idea. You should not store the password itself, but rather a representation of the password.
To do this you must hash the password by saving it in the database and then verify the password passed in the form with the hash to do the verification (in PHP you could use
password_verify
).The advantage of using hashes over encryption algorithms is that by encrypting a password there may be ways to decrypt it; On the other hand, with hashing, there is no going back. If the database were to be compromised, at least the user's passwords wouldn't have been lost (I know it may not be much of a consolation to you, but to the user it may be considering that many use the same passwords from one site to another ).
Apart from that, some comments could be made on how database connections seem to be handled (connecting and disconnecting with each request), although it would be more performance than security.
And some other things that you can't see in the shared code would also need to be evaluated. For example: do you use HTTPS for login? How is the data stored in the database?
You can find more security recommendations on the OWASP page (and in particular for sessions ).