HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Preventing session hijacking

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
preventinghijackingsession

Problem

I'm implementing a login script and I want it to be more secure:

function sec_session_start() {
        $session_name = 'sec_session_id';   // Set a custom session name
        $secure = SECURE;
        // This stops JavaScript being able to access the session id.
        $httponly = true;
        // Forces sessions to only use cookies.
        if (ini_set('session.use_only_cookies', 1) === FALSE) {
            //header("Location: ../error.php?err=Could not initiate a safe session (ini_set)");
            exit();
        }
        // Gets current cookies params.
        $cookieParams = session_get_cookie_params();
        session_set_cookie_params($cookieParams["lifetime"], $cookieParams["path"], $cookieParams["domain"], $secure, $httponly);
        // Sets the session name to the one set above.
        session_name($session_name);
        session_start();            // Start the PHP session 
        session_regenerate_id();    // regenerated the session, delete the old one. 
    }

    function  checkIp($mysqli) {
        if(isset($_SESSION['user_id'])){
            $user_id = $_SESSION['user_id'];
            $user_ip = $_SERVER['REMOTE_ADDR'];
            $user_browser = $_SERVER['HTTP_USER_AGENT'];

            if($stmt === $mysqli->prepare("SELECT UserIP, BrowserInfo FROM users WHERE UserID = ? LIMIT 1")){
                $stmt->bind_param('i', $user_id);
                $stmt->execute();
                $stmt->store_result();

                if($stmt->num_rows == 1){
                    $stmt->bind_result($db_ip,$db_browser);
                    $stmt->fetch();

                    if($user_ip == $db_ip && $user_browser == $db_browser){
                        return true;
                    }else{
                        return false;
                    }
                }else{
                    return false;
                }
            }else{
                return false;
            }
        }else{
            return false;
        }
    }

Solution

Checking an "constant" IP address is an bad idea because of ISP with load balancing. IP address should change to less busy proxy.

In most of cases IP address belongs to the same block. It is called CIDR matching:

function cidr_match($ip, $range)
{
    list ($subnet, $bits) = explode('/', $range);
    $ip = ip2long($ip);
    $subnet = ip2long($subnet);
    $mask = -1 << (32 - $bits);
    $subnet &= $mask; # nb: in case the supplied subnet wasn't correctly aligned
    return ($ip & $mask) == $subnet;
}


usage:

if(!empty($_SESSION['ip'])){
  //192.168.001.xxx = 192.168.001.000/24
  if(!cidr_match($_SERVER['REMOTE_ADDR'],$_SESSION['ip'].'/24')){
     session_destroy();
  }
} else {
  $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
}


http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing

Personally, i recommend to you to check IP address range and some client data (but data may be fake).

PS: do not store session data in MySQL, you should store them just in session.

Code Snippets

function cidr_match($ip, $range)
{
    list ($subnet, $bits) = explode('/', $range);
    $ip = ip2long($ip);
    $subnet = ip2long($subnet);
    $mask = -1 << (32 - $bits);
    $subnet &= $mask; # nb: in case the supplied subnet wasn't correctly aligned
    return ($ip & $mask) == $subnet;
}
if(!empty($_SESSION['ip'])){
  //192.168.001.xxx = 192.168.001.000/24
  if(!cidr_match($_SERVER['REMOTE_ADDR'],$_SESSION['ip'].'/24')){
     session_destroy();
  }
} else {
  $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
}

Context

StackExchange Code Review Q#55558, answer score: 4

Revisions (0)

No revisions yet.