PHP-MySQL sign-in and sign-up project

Question

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

<?php
session_start();
include_once 'dbconnect.php';

if (!isset($_SESSION['userSession'])) {
    header("Location: index.php");
}

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

?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Welcome - <?php echo $userRow['email']; ?></title>

<link href="bootstrap/css/bootstrap.min.css" rel="stylesheet" media="screen"> 
<link href="bootstrap/css/bootstrap-theme.min.css" rel="stylesheet" media="screen"> 

<link rel="stylesheet" href="style.css" type="text/css" />
</head>
<body>

<nav class="navbar navbar-default navbar-fixed-top">
      <div class="container">
        <div class="navbar-header">
          <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#navbar" aria-expanded="false" aria-controls="navbar">
            <span class="sr-only">Toggle navigation</span>
            <span class="icon-bar"></span>
            <span class="icon-bar"></span>
            <span class="icon-bar"></span>
          </button>
          <a class="navbar-brand" href="">Coding Cage</a>
        </div>
        <div id="navbar" class="navbar-collapse collapse">
          <ul class="nav navbar-nav">
            <li class="active"><a href="link1">Back to Article</a></li>
            <li><a href="link2">jQuery</a></li>
            <li><a href="link3">PHP</a></li>
          </ul>
          <ul class="nav navbar-nav navbar-right">
            <li><a href="#"><span class="glyphicon glyphicon-user"></span>&nbsp; <?php echo $userRow['username']; ?></a></li>
            <li><a href="logout.php?logout"><span class="glyphicon glyphicon-log-out"></span>&nbsp; Logout</a></li>
          </ul>
        </div><!--/.nav-collapse -->
      </div>
    </nav>

<div class="container" style="margin-top:150px;text-align:center;font-family:Verdana, Geneva, sans-serif;font-size:35px;">
    <a href="link4/">Coding g</a><br /><br />
    <p>welcome</p>
</div>

</body>
</html>

index.php

<?php
session_start();
require_once 'dbconnect.php';

if (isset($_SESSION['userSession'])!="") {
    header("Location: home.php");
    exit;
}

if (isset($_POST['btn-login'])) {

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

    $email = $DBcon->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 = "<div class='alert alert-danger'>
                    <span class='glyphicon glyphicon-info-sign'></span> &nbsp; Invalid Username or Password !
                </div>";
    }
    $DBcon->close();
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Coding Cage - Login & Registration System</title>
<link href="bootstrap/css/bootstrap.min.css" rel="stylesheet" media="screen">
<link href="bootstrap/css/bootstrap-theme.min.css" rel="stylesheet" media="screen"> 
<link rel="stylesheet" href="style.css" type="text/css" />
</head>
<body>

<div class="signin-form">

    <div class="container">


       <form class="form-signin" method="post" id="login-form">

        <h2 class="form-signin-heading">Sign In.</h2><hr />

        <?php
        if(isset($msg)){
            echo $msg;
        }
        ?>

        <div class="form-group">
        <input type="email" class="form-control" placeholder="Email address" name="email" required />
        <span id="check-e"></span>
        </div>

        <div class="form-group">
        <input type="password" class="form-control" placeholder="Password" name="password" required />
        </div>

        <hr />

        <div class="form-group">
            <button type="submit" class="btn btn-default" name="btn-login" id="btn-login">
            <span class="glyphicon glyphicon-log-in"></span> &nbsp; Sign In
            </button> 

            <a href="register.php" class="btn btn-default" style="float:right;">Sign UP Here</a>

        </div>  



      </form>

    </div>

</div>

</body>
</html>

logout.php

<?php
session_start();

if (!isset($_SESSION['userSession'])) {
    header("Location: index.php");
} else if (isset($_SESSION['userSession'])!="") {
    header("Location: home.php");
}

if (isset($_GET['logout'])) {
    session_destroy();
    unset($_SESSION['userSession']);
    header("Location: index.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 version: 5.6.17
-- PHP Version: 5.5.12

SET SQL_MODE = "NO_AUTO_VALUE_ON_ZERO";
SET time_zone = "+00:00";


/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;
/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;
/*!40101 SET NAMES utf8 */;

--
-- Database: `mysqli_login`
--

-- --------------------------------------------------------

--
-- Table structure for table `tbl_users`
--

CREATE TABLE IF NOT EXISTS `tbl_users` (
  `user_id` int(11) NOT NULL AUTO_INCREMENT,
  `username` varchar(60) NOT NULL,
  `email` varchar(60) NOT NULL,
  `password` varchar(255) NOT NULL,
  PRIMARY KEY (`user_id`),
  UNIQUE KEY `email` (`email`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ;

/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;
/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;

register.php

<?php
session_start();
if (isset($_SESSION['userSession'])!="") {
    header("Location: home.php");
}
require_once 'dbconnect.php';

if(isset($_POST['btn-signup'])) {

    $uname = strip_tags($_POST['username']);
    $email = strip_tags($_POST['email']);
    $upass = strip_tags($_POST['password']);

    $uname = $DBcon->real_escape_string($uname);
    $email = $DBcon->real_escape_string($email);
    $upass = $DBcon->real_escape_string($upass);

    $hashed_password = password_hash($upass, PASSWORD_DEFAULT); // this function works only in PHP 5.5 or latest version

    $check_email = $DBcon->query("SELECT email FROM tbl_users WHERE email='$email'");
    $count=$check_email->num_rows;

    if ($count==0) {

        $query = "INSERT INTO tbl_users(username,email,password) VALUES('$uname','$email','$hashed_password')";

        if ($DBcon->query($query)) {
            $msg = "<div class='alert alert-success'>
                        <span class='glyphicon glyphicon-info-sign'></span> &nbsp; successfully registered !
                    </div>";
        }else {
            $msg = "<div class='alert alert-danger'>
                        <span class='glyphicon glyphicon-info-sign'></span> &nbsp; error while registering !
                    </div>";
        }

    } else {


        $msg = "<div class='alert alert-danger'>
                    <span class='glyphicon glyphicon-info-sign'></span> &nbsp; sorry email already taken !
                </div>";

    }

    $DBcon->close();
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Login & Registration System</title>
<link href="bootstrap/css/bootstrap.min.css" rel="stylesheet" media="screen">
<link href="bootstrap/css/bootstrap-theme.min.css" rel="stylesheet" media="screen"> 
<link rel="stylesheet" href="style.css" type="text/css" />

</head>
<body>

<div class="signin-form">

    <div class="container">


       <form class="form-signin" method="post" id="register-form">

        <h2 class="form-signin-heading">Sign Up</h2><hr />

        <?php
        if (isset($msg)) {
            echo $msg;
        }
        ?>

        <div class="form-group">
        <input type="text" class="form-control" placeholder="Username" name="username" required  />
        </div>

        <div class="form-group">
        <input type="email" class="form-control" placeholder="Email address" name="email" required  />
        <span id="check-e"></span>
        </div>

        <div class="form-group">
        <input type="password" class="form-control" placeholder="Password" name="password" required  />
        </div>

        <hr />

        <div class="form-group">
            <button type="submit" class="btn btn-default" name="btn-signup">
            <span class="glyphicon glyphicon-log-in"></span> &nbsp; Create Account
            </button> 
            <a href="index.php" class="btn btn-default" style="float:right;">Log In Here</a>
        </div> 

      </form>

    </div>

</div>

</body>
</html>

PDO works with whatever mysqli works on. And honestly, it's only difficult if you are re-learning it after learning bad, old habits, like using input sanitation instead of prepared statements. If this is really the first time you are doing SQL on PHP, go here. — Nelson, Commented Oct 3, 2016 at 9:30
I'm dead serious about your method of DB access, because it is obsolete out the gate. You do not want to waste time learning obsolete practices. — Nelson, Commented Oct 3, 2016 at 9:31
@Nelson: mysqli is not deprecated, or obsolete. It's not synonymous with old, bad habits. You can write safe, good, and modern code using mysqli all the same. It supports prepared statements, OOP, and all the rest of it as much as PDO does. that said, though: PDO just is easier to use when you want to do the right thing, and use prepared statements. — Elias Van Ootegem, Commented Oct 3, 2016 at 18:16
Comments have been deleted. Please refrain from recruiting individual reviewers to be your personal tutor. You can ask a follow-up question if you have revised code that you would like to have reviewed. — 200_success, Commented Oct 4, 2016 at 6:59

tim · Accepted Answer · 2016-10-03 09:55:24Z

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.

If you are used to using PDO statement, you will permanently close off this extremely serious security hole. If you always have to think about sanitation, I will guarantee you that you will push out crappy code out due to speed, and this speed will cost you, whereas making PDO prepared statements would've been just as fast to begin with. — Nelson, Commented Oct 3, 2016 at 9:45
@Nelson Exactly. But even if OP wants to stick with mysqli, they can - and really should - still use prepared statements with mysqli. — tim, Commented Oct 3, 2016 at 9:48
i thought mysqli dont support for prepared statements as per this link , @Nelson is that okay if we go with prepared statements in mysqli only, i am only worrying that if i use pdo it may be bit diffucult as i am beginner in php..... — USer345738380, Commented Oct 3, 2016 at 9:54
I find PDO much, MUCH easier than mysqli... and I started with mysql then had to learn mysqli then re-learned PDO, but of course, you learn what you're comfortable with. Just learn prepared statements regardless, and also look at some of the PDO samples. It's crazy how much easier it is to code parsing loops with PDO than mysqli. — Nelson, Commented Oct 3, 2016 at 9:55
btw, that article is 4 years old, so you need better sources. — Nelson, Commented Oct 3, 2016 at 9:55

André Borie · Accepted Answer · 2016-10-03 14:41:33Z

Stop right there.

Whatever you're doing is bad practice, is unmaintainable in the long run, has tons of security flaws, is impossible to test and would make any competent developer cry.

I suggest you start fresh with some proper documentation - forget the PHP "tutorials" written by clueless kids which sadly are way too common on the Internet.

That document is a long read, but the main thing I'd recommend is using a framework. They include the common boilerplate needed to start an app, so you can actually write your app's code instead of the supporting code (ORM, sessions, logins, etc).

There are plenty to choose from, and must of them would provide the functionality you need for user registration and login right out of the box - the only code you'll need to write would be templates.

Frameworks will also be way more convenient (database access using model objects instead of queries), will clearly follow a design pattern (most often the Model View Controller one) so the code will be more readable, have a test infrastructure ready so you can test your code easily, and more.

Please don't be like many people who learn on W3fools, configure database access by including a file and call themselves developers while not having even the slightest idea how software development is done properly. I don't want to see you on IT Security SE tomorrow with "Help! My new PHP site got hacked!", we have enough of those question as it is.

Good luck!

Ferrybig · Accepted Answer · 2016-10-03 12:32:08Z

You're including mysql specific data in your passwords.

$password = $DBcon->real_escape_string($password);
password_verify($password, $row['password'])

If you ever change the charset of the connection, and the user uses special characters in his password, it means he cannot login anymore.

This also makes mysql a dependency of your passwords, this limits yourself to always using an mysql specific library for quoting/escaping the passwords, effectively locking yourself (and the passwords) to that platform

Stack Exchange Network

PHP-MySQL sign-in and sign-up project

3 Answers 3

Security

You're including mysql specific data in your passwords.

Your Answer

Not the answer you're looking for? Browse other questions tagged
performance
php
html
mysql
authentication
or ask your own question.

Hot Network Questions

PHP-MySQL sign-in and sign-up project

3 Answers 3

Security

You're including mysql specific data in your passwords.

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged performancephphtmlmysqlauthentication or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
performance
php
html
mysql
authentication
or ask your own question.