CODEWITHSUNDEEP
phpcookiesauthenticationsecuritysession-variables
user701510
user701510

Reputation: 5763

can I improve my PHP log-in system with cookies?

I'm currently testing a simple log-in system I've made. What the script does is it checks the username and password I've entered with the ones in my database and if they both match, it sets the username to a session variable. When I go to a "members only" page, it will check to see if the session variable exists, if it does not exist, the user cannot view the content. It seems to work as intended. Based on the above info, here are my questions:

  1. What vulnerabilities/flaws are there to my method?

  2. Can cookies make it more secure? If so, how would it do so with relation to my code?

Here's what I'm working with so far:

username and password check:

<?
session_start();

$username = $_POST['username'];
$password = $_POST['password'];

if ($username && $password){
$query = mysql_query("SELECT * FROM user WHERE username='$username' ");
$numrows = mysql_num_rows($query);

    //if user exists
    if ($numrows !=0){
        while ($row = mysql_fetch_array($query)){
                $dbusername = $row['username'];
                $dbpassword = $row['password'];
            }

        if ($username == $dbusername && md5($password) == $dbpassword){
            echo 'You\'re in! <a href="member.php">Click</a> here to enter the member page.';   
            $_SESSION['username'] = $dbusername;

        }
        else
            echo "incorrect password!";
    }
    else
        die("sorry, that user doesn't exist!");
}



?>

members only page:

<?
session_start();


if ($_SESSION['username']){
echo "Welcome, ".$_SESSION['username']."!<br>
<a href='changepassword.php'>Change password</a>
";
}
else{
echo "please log in";   
}
?>

Upvotes: 0

Views: 1704

Answers (2)

Yoone
Yoone

Reputation: 549

I recommand you to use the PDO class of PHP for your queries, for many reasons:

  • If you change your DBMS, you wouldn't have to edit all your mysql_query(), mysql_numrows(), etc.
  • For security reasons, because PDO has some pre-integrated functions to prevent from security issues.
  • ...

So, with PDO, your code would be like this:

<?php
session_start();

$db = new PDO('mysql:host='.DB_HOST.';dbname='.DB_NAME, DB_USER, DB_PASS);

$username = $_POST['username'];
$password = $_POST['password'];

if ($username && $password){
$query = $db->prepare("SELECT * FROM user WHERE username = :usr");
$query->execute(array('usr' => $username));
$numrows = $query->rowCount();

    //if user exists
    if ($numrows !=0){
        while ($row = $query->fetch()){
                $dbusername = $row['username'];
                $dbpassword = $row['password'];
            }

        if ($username == $dbusername && md5($password) == $dbpassword){
            echo 'You\'re in! <a href="member.php">Click</a> here to enter the member page.';   
            $_SESSION['username'] = $dbusername;

        }
        else
            echo "incorrect password!";
    }
    else
        die("sorry, that user doesn't exist!");
}
?>

The method prepare() prevents from SQL injections. execute() is needed after a prepare(), but you can use the method query() without execute().

The fetch() method is the equivalent to mysql_fetch_array().

Here is the PDO doc: http://www.php.net/manual/en/book.pdo.php

Hope I helped.

NB: I prefer <?php instead of <?.

Upvotes: 1

regality
regality

Reputation: 6554

Presently the biggest security issue is SQL injection which is a massive flaw in the current script. Anybody could log in without any credentials with the script you have posted. Change this line:

$username = $_POST['username'];

to look like:

$username = mysql_real_escape_string($_POST['username']);

As far as cookies go, sessions use cookies, so you are already using them. Secondly, anything you put in a cookie gets transferred over the wire, so they are not secure.

Other security measures that would be good is to use bcrypt or at least a sha2 variant, instead of md5 for hashing passwords.

It is also preferable to hash passwords using javascript before you send them over the wire, this is extremely important if you are not using SSL.

That'll get you started. After you take care of these problems you can move on to CSRF and session hijacking attacks.

Upvotes: 5

Related Questions