Always authenticate the user before you take any restricted action.
In login.php you have:
session_start();
if(!empty($_SESSION['username'])) {
header('location:plan.php');
}
This code has 2 vulnerabilities I can think of:
It assumes that the existence of username
in the session is proof of authenticity. If your user doesn't log out, anyone who uses his browser much later to come to your site will be admitted; likewise anyone who logs HTTP requests and later looks at the log will find the session cookie and can impersonate the user. To fix, store a valid_until
date in the session when the user logs in. Then when you check for credentials, you only admit the user if the login validity has not expired.
Your site is vulnerable to Cross-Site Request Forgery (CSRF). Suppose your logged-in user visits my website where I have something like:
.
<--! Make user send request to your site without knowing -->
<img src="http://yoursite.com/plan.php?do=evil">
Because browsers send cookies automatically with every request, the user's session
cookie will be sent to plan.php
when the browser tries to load the image, and your script will treat the request as legitimate, therefore it will act on ?do=evil
. To fix, set a token in your response headers when the user logs in, and expect the same token back from the request headers from legitimate requests.
The two points above lead to a modification of login.php
. When the user logs in, you now do something like:
$_SESSION['username'] = $user;
$_SESSION['valid_until'] = time() + 60*60; //1hr from now
$_SESSION['csrf_token'] = 'long_random_string'; //research how to build this
header("X-CSRF: $_SESSION[csrf_token]");
header('location:plan.php');
When your user comes to my evil site and I try to connect to plan.php
, the browser will still send the session
cookie but it will not send the CSRF token, so before you grant access you would check for it:
session_start();
$logged_in = false;
if(isset($_SESSION['username'])){
if(!$_SESSION['valid_until'] >= time()) $logged_in = false;
$request_headers = apache_request_headers();
$csrf = isset($request_headers["X-CSRF"])? $request_headers["X-CSRF"] : null;
if($_SESSION['csrf_token']!==$csrf) $logged_in = false;
}
if($logged_in){/*give access*/}
else{/*block access*/}
By the way, this logic should be moved to a function that returns TRUE if the user is logged in, false otherwise. You can extend it with further checks. For instance you could record the IP address to which the session was granted, and if it doesn't match the IP of the request, deny access (to protect against an attacker stealing the session and CSRF token and trying to re-use it from another location).
In login.php you have:
$user = $_POST['username'];
$pass = $_POST['password'];
...
$query = $conn->prepare("SELECT username, password FROM users WHERE
username=? AND password=? ");
$query->execute(array($user,$pass));
It's great that you use prepared statements to prevent SQL Injection, but this code is evidence that you're storing the passwords in clear text, without hashing. This means that if your database gets stolen, the thief will know your users' credentials, and can use them to impersonate the users on other services. It also means that anyone with access to the database can do the same thing. Instead, when a user registers, hash the password before saving it.
$password = password_hash($_POST['password'],PASSWORD_DEFAULT);
Then when a user logs in, you verify the submitted password against the hashed password. Do something like:
$query = $conn->prepare("SELECT username, password FROM users WHERE username=?");
... // <-execute query and capture password in $db_pass
if($query->rowCount() > 0 && password_verify($_POST['password'],$db_pass)){
//credentials are correct
}else{/*user unknown or password incorrect*/}