Session ID: extend the regex to match possible hash representations

Improves #306
Relates to #335 & #336
Duplicated by #339

Issues:
 - PHP regenerates the session ID if it is not compliant
 - the regex checking the session ID does not cover all cases
   - different algorithms: md5, sha1, sha256, etc.
   - bit representations: 4, 5, 6

Fix:
 - `index.php`:
   - remove `uniqid()` usage
   - call `session_regenerate_id()` if an invalid cookie is detected
 - regex: support all possible characters - '[a-zA-Z,-]{2,128}'
 - tests: add coverage for all algorithms & bit representations

See:
 - http://php.net/manual/en/session.configuration.php#ini.session.hash-function
 - https://secure.php.net/manual/en/session.configuration.php#ini.session.hash-bits-per-character
 - http://php.net/manual/en/function.session-id.php
 - http://php.net/manual/en/function.session-regenerate-id.php
 - http://php.net/manual/en/function.hash-algos.php

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
This commit is contained in:
VirtualTam 2015-09-03 23:12:58 +02:00
parent a02257b8ae
commit 68bc21353a
4 changed files with 119 additions and 9 deletions

View file

@ -140,11 +140,16 @@ function checkPHPVersion($minVersion, $curVersion)
/**
* Validate session ID to prevent Full Path Disclosure.
*
* See #298.
* The session ID's format depends on the hash algorithm set in PHP settings
*
* @param string $sessionId Session ID
*
* @return true if valid, false otherwise.
*
* @see http://php.net/manual/en/function.hash-algos.php
* @see http://php.net/manual/en/session.configuration.php
*/
function is_session_id_valid($sessionId)
{
@ -156,7 +161,7 @@ function is_session_id_valid($sessionId)
return false;
}
if (!preg_match('/^[a-z0-9]{2,32}$/i', $sessionId)) {
if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
return false;
}

View file

@ -92,16 +92,18 @@
// Prevent PHP form using sessionID in URL if cookies are disabled.
ini_set('session.use_trans_sid', false);
// Regenerate session id if invalid or not defined in cookie.
if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) {
$_COOKIE['shaarli'] = uniqid();
}
session_name('shaarli');
// Start session if needed (Some server auto-start sessions).
if (session_id() == '') {
session_start();
}
// Regenerate session ID if invalid or not defined in cookie.
if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) {
session_regenerate_id(true);
$_COOKIE['shaarli'] = session_id();
}
include "inc/rain.tpl.class.php"; //include Rain TPL
raintpl::$tpl_dir = $GLOBALS['config']['RAINTPL_TPL']; // template directory
raintpl::$cache_dir = $GLOBALS['config']['RAINTPL_TMP']; // cache directory

View file

@ -4,12 +4,28 @@
*/
require_once 'application/Utils.php';
require_once 'tests/utils/ReferenceSessionIdHashes.php';
// Initialize reference data before PHPUnit starts a session
ReferenceSessionIdHashes::genAllHashes();
/**
* Unitary tests for Shaarli utilities
*/
class UtilsTest extends PHPUnit_Framework_TestCase
{
// Session ID hashes
protected static $sidHashes = null;
/**
* Assign reference data
*/
public static function setUpBeforeClass()
{
self::$sidHashes = ReferenceSessionIdHashes::getHashes();
}
/**
* Represent a link by its hash
*/
@ -152,11 +168,41 @@ public function testCheckSupportedPHPVersion52()
}
/**
* Test is_session_id_valid with a valid ID.
* Test is_session_id_valid with a valid ID - TEST ALL THE HASHES!
*
* This tests extensively covers all hash algorithms / bit representations
*/
public function testIsSessionIdValid()
public function testIsAnyHashSessionIdValid()
{
$this->assertTrue(is_session_id_valid('azertyuiop123456789AZERTYUIOP1aA'));
foreach (self::$sidHashes as $algo => $bpcs) {
foreach ($bpcs as $bpc => $hash) {
$this->assertTrue(is_session_id_valid($hash));
}
}
}
/**
* Test is_session_id_valid with a valid ID - SHA-1 hashes
*/
public function testIsSha1SessionIdValid()
{
$this->assertTrue(is_session_id_valid(sha1('shaarli')));
}
/**
* Test is_session_id_valid with a valid ID - SHA-256 hashes
*/
public function testIsSha256SessionIdValid()
{
$this->assertTrue(is_session_id_valid(hash('sha256', 'shaarli')));
}
/**
* Test is_session_id_valid with a valid ID - SHA-512 hashes
*/
public function testIsSha512SessionIdValid()
{
$this->assertTrue(is_session_id_valid(hash('sha512', 'shaarli')));
}
/**
@ -166,6 +212,8 @@ public function testIsSessionIdInvalid()
{
$this->assertFalse(is_session_id_valid(''));
$this->assertFalse(is_session_id_valid(array()));
$this->assertFalse(is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI='));
$this->assertFalse(
is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
);
}
}

View file

@ -0,0 +1,55 @@
<?php
/**
* Testing the untestable - Session ID generation
*/
class ReferenceSessionIdHashes
{
// Session ID hashes
protected static $sidHashes = null;
/**
* Generates session ID hashes for all algorithms & bit representations
*/
public static function genAllHashes()
{
foreach (hash_algos() as $algo) {
self::$sidHashes[$algo] = array();
foreach (array(4, 5, 6) as $bpc) {
self::$sidHashes[$algo][$bpc] = self::genSidHash($algo, $bpc);
}
}
}
/**
* Generates a session ID for a given hash algorithm and bit representation
*
* @param string $function name of the hash function
* @param int $bits_per_character representation type
*
* @return string the generated session ID
*/
protected static function genSidHash($function, $bits_per_character)
{
if (session_id()) {
session_destroy();
}
ini_set('session.hash_function', $function);
ini_set('session.hash_bits_per_character', $bits_per_character);
session_start();
return session_id();
}
/**
* Returns the reference hash array
*
* @return array session IDs generated for all available algorithms and bit
* representations
*/
public static function getHashes()
{
return self::$sidHashes;
}
}