Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Am I over reacting with my SESSIONS security?

Tags:

php

mysql

session

In order to secure my page SESSIONS I have the following pages.

My questions are

  1. Am I over-reacting with this?
  2. Should I place the token in login.php instead of the loginForm.php ?
  3. When a user logins, I save his IP in the DB. Should I make use of this in authentication ?

Thank you community.

Login form loginForm.php

$token = md5(uniqid(rand(),TRUE));
<input name="login" type="text" class="textfield" id="login" />
<input name="password" type="password" class="textfield" id="password" />
<input type="hidden" name="token" value="<?php echo $token; ?>" />
<input type="submit" name="Submit" value="Login" />

When the user logins login.php

$fingerprint = sha1('SECRET-SALT'.$_SERVER['HTTP_USER_AGENT'].$_SERVER['REMOTE_ADDR'].$_POST['token']);
    session_regenerate_id();
    $member = mysql_fetch_assoc($result);
$_SESSION['SESS_MEMBER_ID'] = $member['member_id'];
$_SESSION['SESS_TOKEN'] = $_POST['token'];
$_SESSION['SESS_FINGERPRINT'] = $fingerprint;
session_write_close();
    header("location: index.php");
    exit();

Authenticating on every page auth.php

    session_start();
    $fingerprint = sha1('SECRET-SALT'.$_SERVER['HTTP_USER_AGENT'].$_SERVER['REMOTE_ADDR'].$_SESSION['SESS_TOKEN']);
    if( !isset($_SESSION['SESS_MEMBER_ID']) || (trim($_SESSION['SESS_MEMBER_ID']) == '')  || ($_SESSION['SESS_FINGERPRINT'] != $fingerprint) || !isset($_SESSION['SESS_TOKEN']) || (trim($_SESSION['SESS_TOKEN']) == '') ) {
        header("location: denied.php");
        exit();
    }
like image 811
EnexoOnoma Avatar asked Aug 24 '11 16:08

EnexoOnoma


2 Answers

To answer your top 3 questions:

  1. Am I over-reacting with this?

    Nope. Broken session management is number 3 on OWASP's Top 10 Vulnerability List. There are a few issues with your implementation, but in general it's a good idea.

  2. Should I place the token in login.php instead of the loginForm.php ?

    Kind of. You should place the token generation into the session directly. Then, insert it into the form from the session. That way you can verify that the form submit was the same session that requested the form (can prevent certain attacks). It's basically preventing CSRF attacks.

  3. When a user logins, I save his IP in the DB. Should I make use of this in authentication ?

    No. Users will often log in from multiple computers. There is no IP->user mapping that's really going to be possible. IP->session mapping may work better, but don't forget that a fair number of users are behind a dynamic IP address, so it may not be reliable even for sessions.

And here are some comments on your code itself:

  • Token generation. Right now, you're using the following algorithm:

    $token = md5(uniqid(rand(),TRUE));
    

    I would personally change that to something a little more random and secure. Something such as:

    $token = md5(uniqid(mt_rand() . mt_rand(), true);
    

    Or, if you have mcrypt installed, I would do this instead:

    $token = md5(mcrypt_create_iv(32, MCRYPT_DEV_URANDOM));
    

    The reason is that both of those functions have a significant amount more entropy (meaning they will generate a large number of unique values), and they are more difficult for an attacker to guess.

  • The Fingerprint. Right now, you're doing:

    $fingerprint = sha1(
        'SECRET-SALT' . 
        $_SERVER['HTTP_USER_AGENT'] . 
        $_SERVER['REMOTE_ADDR'] . 
        $_POST['token']
    );
    

    There are a few problems with this. First of all, dynamic IP addresses will invalidate a session erroneously. Secondly, there's no real reason to hash this data. Keep each one as a separate field, and then verify them individually. It can help prevent a certain theoretically attack where a user with a different remote IP can create a forged token and useragent which results in the same hash (and hence is able to "crack" the fingerprint). Third, you are using the inputted token as identifying data. This can lead to session fixation style attacks.

    What I would suggest, is storing each piece of information in its own session variable, and verifying them individually. Additionally, use the session token that you generated on the previous request, and only verify that the posted token matches the one you saved.

  • The Session Validation. That's pretty good there. I would suggest moving it out to a separate function so that it's easy to modify if you need to change the algorithm. That way you would just call:

    require_once 'session.php';
    verifySession();
    

    At the top of each file...

What you have is clearly procedural, and can benefit from some design and abstraction, but for the most part it's pretty good (the above comments aside).

like image 161
ircmaxell Avatar answered Nov 18 '22 12:11

ircmaxell


The first thing you should do is to manage session hijacking. This means session_regenerate_id() when the user logs in/out. After that, you should consider using https.

like image 2
Gabi Purcaru Avatar answered Nov 18 '22 12:11

Gabi Purcaru