Properly check whether the user is logged in

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
MIHAIO
Forum Newbie
Posts: 7
Joined: Thu Nov 27, 2008 11:46 pm

Properly check whether the user is logged in

Post by MIHAIO »

Hello,

Check the logo credentials and store the id of the user in a session

Code: Select all

 
<?php
include("con.php");
session_start();
 
$user       = mysql_real_escape_string($_POST['user']);
$password   = md5($_POST['password']);
 
$sql_check = "SELECT id 
                     from users where 
                          username = '$user' and
                          password = '$password'";
  $result = mysql_query($sql_check); // trigger_error ('Query failed: '. mysql error());
        if (mysql_num_rows($result) !=1)
        {
            header('Location: log.php'); // re-direct the user to the login page
        }
        else
        {
 
         $id = mysql_result($result,0);
         $_SESSION['id_user'] = $id;
 
            header('Location: welcome.php'); // enter the site 
        }
?>
 
In the pages that the user shouldn't be if he is not logged in I have at the top

Code: Select all

 
<?php
session_start();
if (is_null($_SESSION["id_user"])){
    header('Location: log.php');
}


Then I use the id stored in the session to read/edit various information .

It works but I have been reading quite a lot on this forum about this subject and it seems my approach is not too safe and a better way would be:

1. If the user/password match what is stored in the database save the ID of the user , Session ID and IP of the computer in the database.

For the Session ID I would use $_COOKIE["PHPSESSID"]
For the IP I would use the example found here http://wiki.jumba.com.au/wiki/Get_user_IP_Address_(PHP)

2. On each page do additional checks beside

Code: Select all

if (is_null($_SESSION["id_user"])

- retrieve Session ID and IP for the $_SESSION["id_user"] from the db
- get again $_COOKIE["PHPSESSID"] and IP and compare with the values retieved from the db

I understand that in the case of a dynamic IP this would cause a problem as users would be disconnected but somehow I am willing to take this risk.

Any thoughts about this approach ?

Thanks,
User avatar
sergio-pro
Forum Commoner
Posts: 88
Joined: Sat Dec 27, 2008 12:26 pm

Re: Check whether the user is logged in

Post by sergio-pro »

Hi

IMHO, saving and checking SessionID does not make sence.

About IP check, the problem can happen here - is that another user opens page with current user's SessionID.
This can be (potentially) in two cases:
1. Same SessionID generated by server for two users.
2. Hacker intercepted user's SessionID - and uses it to acces the system.

The first is very unlikely.
The second, and many other issues, can be avoided by using https.
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: Properly check whether the user is logged in

Post by Mordred »

Your original approach is more or less okay, with the following notes:
1. regenerate_session_id() after login to avoid session fixation attacks
2. exit() after header('Location...) (in all instances)

If you want to store additional things, you can do it in the session itself - it's just a storage mechanism. Don't rely on IP, it can change between requests.

sergio-pro is wrong in his point 2, https only prevents an attacker to obtain the SID by sniffing traffic, there are other ways (xss, fixation) and https will not help with them.
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: Properly check whether the user is logged in

Post by kaisellgren »

MIHAIO wrote:

Code: Select all

 
<?php
session_start();
if (is_null($_SESSION["id_user"])){
    header('Location: log.php');
}
If the id_user is empty, the execution will still continue and that might not be good. Like Mordred said, exit().

Code: Select all

 
<?php
session_start();
if (is_null($_SESSION["id_user"])){
    header('Location: log.php');
    exit();
}
Post Reply