PHP-MySQL sign-in and sign-up project

I have done my first PHP-MySQL project with MySQLi connection. Please review this and inform me about security and performance issues.

dbconnect.php

$DBhost = "localhost";
$DBuser = "root";
$DBpass = "";
$DBname = "mysqli_login1";

$DBcon = new MySQLi($DBhost,$DBuser,$DBpass,$DBname);

if ($DBcon->connect_errno) {
die("ERROR : -> ".$DBcon->connect_error);
}


home.php

query("SELECT * FROM tbl_users WHERE user_id=".$_SESSION['userSession']);
$userRow=$query->fetch_array();
$DBcon->close();

?>

Welcome -







Toggle navigation




Coding Cage



Back to Article

• jQuery

• PHP

•  

•   Logout

Coding g


welcome



index.php

real_escape_string($email);
$password = $DBcon->real_escape_string($password);

$query = $DBcon->query("SELECT user_id, email, password FROM tbl_users WHERE email='$email'");
$row=$query->fetch_array();

$count = $query->num_rows; // if email/password are correct returns must be 1 row

if (password_verify($password, $row['password']) && $count==1) {
$_SESSION['userSession'] = $row['user_id'];
header("Location: home.php");
} else {
$msg = "
  Invalid Username or Password !
";
}
$DBcon->close();
}
?>

Coding Cage - Login & Registration System







Sign In.
















  Sign In


Sign UP Here









logout.php



login.sql

-- phpMyAdmin SQL Dump
-- version 4.1.14
-- http://www.phpmyadmin.net
--
-- Host: 127.0.0.1
-- Generation Time: Aug 14, 2016 at 08:16 PM
-- Server v

Security

There are quite a lot of beginners mistakes in your code. While there aren't any serious security issues right now, it does contain a whole bunch of bad practices which will sooner or later lead to security issues.

If you follow some tutorial which suggested these approaches you used, I would suggest to use a more up-to-date tutorial which places at least some considerations on security.

SQL Injection

You are not currently vulnerable to SQL injection, but if you continue using escaping instead of prepared statements, you will make a mistake sooner or later, and you will be vulnerable.

There is no reason not to use prepared statements. They are a lot safer, result in easier to read code, and they are easy to use.

When you use prepared statements, you will want to bind all variable data, not just data you suspect to be insecure. If you need to think about it each time, you will make a mistake at some point.

XSS

While you do not currently seem to be vulnerable, just applying strip_tags to all input is not a proper defense against XSS. XSS is an output vulnerability, and that is where it needs to be defended. XSS is also context sensitive, so strip_tags may not be enough, depending on context.

Ideally, you would use a templating engine that HTML escapes by default, and JS encodes when necessary. Alternatively, apply htmlspecialchars with ENT_QUOTES when echoing any variables (except in a JS context, where you want to JS encode).

CSRF

You do not seem to have any CSRF protection. If you didn't just omit that part of your code, you should add protection against CSRF.

Weak Passwords

By applying strip_tags to all your input, you are modifying user input in a way you shouldn't be. This can have effects on usability (eg it may make a valid email address invalid), but it also has a negative impact on security. My favorite password i<3My_Secure!Password becomes i with your script.

Header Redirect

You always need to die after a header redirect, as a client does not have to follow it. All following code will thus be executed.

There doesn't seem to be any danger from this in your current code, but it is a very bad practice which will lead to issues later.

Relative Path Include

You generally don't want to include CSS or JS files relative, but absolute. Otherwise, it will lead to relative path overwrite, which may lead to CSS injection (or even XSS, in some limited circumstances). Depending on the document type, only older browsers are affected, but it's still better to do it right.

Error Messages

Don't echo error messages to the enduser. It's information that they don't need, and it may leak some data (eg the database username in this case; in general, it may also enable error based SQL injection, which would be an improvement over blind SQL injection).

Session Management

You should regenerate the session id whenever the session state changes (eg when you log in). This will prevent session fixation vulnerabilities which occur with some non-default PHP configurations, or which may be introduced by vulnerable code elsewhere.

StackExchange Code Review Q#143074, answer score: 10