PHP PDO login with session

Question

index.php,general message.php, logout.php, site life.php (this page for session and put it in the other pages by required)

Database:

connection.php

<?php 
$dsn = "mysql:host=localhost;dbname=mg";
$username = "root";
$password = "";
$options = array(
PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
);

try{
$conn = new PDO($dsn,$username,$password,$options);
} catch (PDOException $e){
echo "Error!".$e->getMessage();
}

?>

index.php:

<?php
session_start();
if(isset($_SESSION['user'])){
header("location: general message.php");
}
require "connection.php";

if(isset($_POST['login'])){
$user = $_POST['username'];
$pass = md5($_POST['password']);
$messeg = "";

if(empty($user) || empty($pass)) {
    $messeg = "Username/Password con't be empty";
} else {
    $sql = "SELECT username, password FROM users WHERE username=? AND 
  password=? ";
    $query = $conn->prepare($sql);
    $query->execute(array($user,$pass));

    if($query->rowCount() >= 1) {
        $_SESSION['user'] = $user;
        $_SESSION['time_start_login'] = time();
        header("location: general message.php");
    } else {
        $messeg = "Username/Password is wrong";
    }
}
}

?>

Site life.php (and I will put it in the the other pages by require "site life.php")

//The lives of session is one hour 60*60=3600 

<?php
session_start();

if(isset($_SESSION['user'])){
if((time() - $_SESSION['time_start_login']) > 3600){
    header("location: logout.php");
} else {
    $_SESSION['time_start_login'] = time();
}
} else {
header("location: logout.php");
}
 ?>

logout.php

<?php
session_start();
session_destroy();
header("location: index.php");
?>

General message.php

I put this in the header (to make a refresh every hour):

// 60*60=3600 one hour

<meta http-equiv="Refresh" content="3600" >

<?php 
require ('site life.php');
?>

Community · Accepted Answer · 2020-06-10 13:24:26Z

Starting a database session

You are starting a dabatase session with the following code:

try{
$conn = new PDO($dsn,$username,$password,$options);
} catch (PDOException $e){
echo "Error!".$e->getMessage();
}

It is unclear to me why you let the request live on. If the application needs the database connection, letting the request live on produces all kinds of behaviour from code that assumes the connection is there. Consider killing the request right there and then.

Besides this, don't print the error message to the user. It is not useful to the average user, and might give an adversary additional information to find weaknesses in your site.

Keep in mind that if you include a php file, the variables inside it are available even outside that file. If an adversary can trick any other file that includes connection.php to display the contents of the $password variable, they can obtain the password this way. Arguably, the chance of this happening, but the adversary not being able to execute arbitrary code, is very small. You could defend yourself against it by unsetting the variable, or define those variables only in a function.

Storage of password

You are storing an unsalted MD5 hash from the password. MD5 has not been crytographically secure for a long time now, and any password can easily be brute-forced in it. Since your password is unsalted, cracking the passwords is as simple as looking it up in multiple already existing Rainbow tables. For example, the MD5 hash for your account is "Mohamed" (http://md5decoder.org/bc2ab28e4cda984ca76646874371e864).

I recommend to use bcrypt instead as hash function. It is designed to be slow, and the algorithm can not easily be sped up by implementing it on a GPU. See this question on security.SE for more information.

You should always add a per-user salt. By using an unique salt for each user, brute-forcing the password of one user will not yield any useful results for any other user. If no such salt were to be implemented, we could build a rainbow table for each password that we tried, and simply look up in the table for other users. A salt should be reasonably random, and sufficiently long. See this question on Stackoverflow for more information.

Checking of password

There are a few things I want to say about the following code.

if($query->rowCount() >= 1) {

->rowCount() is not guaranteed to return the number of rows in the result for select statements as per the documentation, quoted below. A possible safer alternative is count($query->fetch_all()).

If the last SQL statement executed by the associated PDOStatement was a SELECT statement, some databases may return the number of rows returned by that statement. However, this behaviour is not guaranteed for all databases and should not be relied on for portable applications.

I do not understand why you check for >= 1 in this code. Only one user/password should ever match. If this is not the case, something very bad happened. Either rowCount returns something that has nothing to do with this select statement, someone tries to get the query to return more information or your data is corrupt somehow. In either case, you want to know about this, not silently log in someone when more than one row is returned.

Misc

Use consistent indentation in your code. Inside your try block you don't indent. Similarly, your nested if-statements are not indented nicely.

Dear Sumurai8: First: Did you mean that I do not have to use try and catch and as long as the communication process was successful, I have to use the code as follows: <?php $dsn = "mysql:host=localhost;dbname=mg"; $username = "root"; $password = ""; $options = array( PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8', ); — Mohamed Ahmed
– Mohamed Ahmed, Commented Jul 10, 2016 at 16:13
I do not understand why you check for >= 1 ?.... if i have tow user have same username and password what i can do? — Mohamed Ahmed
– Mohamed Ahmed, Commented Jul 10, 2016 at 16:21
I mean that in the catch block you should put a exit() or die() statement, to prevent the code from continuing when connecting failed and thus $conn is not set. — Sumurai8
– Sumurai8, Commented Jul 10, 2016 at 16:21
If you have two users with the same username, how do you know which account you have to log them into? If one of them is an administrator, and one a spambot, do you just log them both in on the first account that appears? This exact problem is why a username has to be unique on a site. On sites where a username does not need to be unique, such as StackExchange, we log in with an unique email address, or unique third-party account. — Sumurai8
– Sumurai8, Commented Jul 10, 2016 at 16:29
If you implement a salt correctly, you need to look up the username first to obtain the salt, then compare the hashed password + salt to the hash you obtained in the same lookup. — Sumurai8
– Sumurai8, Commented Jul 10, 2016 at 16:30

Stack Exchange Network

PHP PDO login with session

1 Answer 1

Starting a database session

Storage of password

Checking of password

Misc

You must log in to answer this question.

Hot Network Questions

PHP PDO login with session

1 Answer 1

Starting a database session

Storage of password

Checking of password

Misc

You must log in to answer this question.

Related

Hot Network Questions