Move session ID check to SessionManager

Relates to https://github.com/shaarli/Shaarli/issues/324

Changed:
- `is_session_id_valid()` -> `SessionManager::checkId()`
- update tests

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
This commit is contained in:
VirtualTam 2017-10-22 19:54:44 +02:00
parent ebd650c06c
commit fd7d84616d
5 changed files with 97 additions and 90 deletions

View file

@ -50,4 +50,34 @@ class SessionManager
unset($this->session['tokens'][$token]);
return true;
}
/**
* 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
*/
public static function checkId($sessionId)
{
if (empty($sessionId)) {
return false;
}
if (!$sessionId) {
return false;
}
if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
return false;
}
return true;
}
}

View file

@ -181,36 +181,6 @@ function generateLocation($referer, $host, $loopTerms = array())
return $finalReferer;
}
/**
* 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)
{
if (empty($sessionId)) {
return false;
}
if (!$sessionId) {
return false;
}
if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
return false;
}
return true;
}
/**
* Sniff browser language to set the locale automatically.
* Note that is may not work on your server if the corresponding locale is not installed.

View file

@ -116,7 +116,7 @@ if (session_id() == '') {
}
// Regenerate session ID if invalid or not defined in cookie.
if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) {
if (isset($_COOKIE['shaarli']) && !SessionManager::checkId($_COOKIE['shaarli'])) {
session_regenerate_id(true);
$_COOKIE['shaarli'] = session_id();
}

View file

@ -1,8 +1,12 @@
<?php
namespace Shaarli;
// Initialize reference data _before_ PHPUnit starts a session
require_once 'tests/utils/ReferenceSessionIdHashes.php';
ReferenceSessionIdHashes::genAllHashes();
use \Shaarli\SessionManager;
use \PHPUnit\Framework\TestCase;
/**
* Fake ConfigManager
*/
@ -20,6 +24,17 @@ class FakeConfigManager
*/
class SessionManagerTest extends TestCase
{
// Session ID hashes
protected static $sidHashes = null;
/**
* Assign reference data
*/
public static function setUpBeforeClass()
{
self::$sidHashes = ReferenceSessionIdHashes::getHashes();
}
/**
* Generate a session token
*/
@ -69,4 +84,54 @@ class SessionManagerTest extends TestCase
$this->assertFalse($sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
}
/**
* Test SessionManager::checkId with a valid ID - TEST ALL THE HASHES!
*
* This tests extensively covers all hash algorithms / bit representations
*/
public function testIsAnyHashSessionIdValid()
{
foreach (self::$sidHashes as $algo => $bpcs) {
foreach ($bpcs as $bpc => $hash) {
$this->assertTrue(SessionManager::checkId($hash));
}
}
}
/**
* Test checkId with a valid ID - SHA-1 hashes
*/
public function testIsSha1SessionIdValid()
{
$this->assertTrue(SessionManager::checkId(sha1('shaarli')));
}
/**
* Test checkId with a valid ID - SHA-256 hashes
*/
public function testIsSha256SessionIdValid()
{
$this->assertTrue(SessionManager::checkId(hash('sha256', 'shaarli')));
}
/**
* Test checkId with a valid ID - SHA-512 hashes
*/
public function testIsSha512SessionIdValid()
{
$this->assertTrue(SessionManager::checkId(hash('sha512', 'shaarli')));
}
/**
* Test checkId with invalid IDs.
*/
public function testIsSessionIdInvalid()
{
$this->assertFalse(SessionManager::checkId(''));
$this->assertFalse(SessionManager::checkId([]));
$this->assertFalse(
SessionManager::checkId('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
);
}
}

View file

@ -5,10 +5,6 @@
require_once 'application/Utils.php';
require_once 'application/Languages.php';
require_once 'tests/utils/ReferenceSessionIdHashes.php';
// Initialize reference data before PHPUnit starts a session
ReferenceSessionIdHashes::genAllHashes();
/**
@ -16,9 +12,6 @@ ReferenceSessionIdHashes::genAllHashes();
*/
class UtilsTest extends PHPUnit_Framework_TestCase
{
// Session ID hashes
protected static $sidHashes = null;
// Log file
protected static $testLogFile = 'tests.log';
@ -30,13 +23,11 @@ class UtilsTest extends PHPUnit_Framework_TestCase
*/
protected static $defaultTimeZone;
/**
* Assign reference data
*/
public static function setUpBeforeClass()
{
self::$sidHashes = ReferenceSessionIdHashes::getHashes();
self::$defaultTimeZone = date_default_timezone_get();
// Timezone without DST for test consistency
date_default_timezone_set('Africa/Nairobi');
@ -221,56 +212,7 @@ class UtilsTest extends PHPUnit_Framework_TestCase
$this->assertEquals('?', generateLocation($ref, 'localhost'));
}
/**
* Test is_session_id_valid with a valid ID - TEST ALL THE HASHES!
*
* This tests extensively covers all hash algorithms / bit representations
*/
public function testIsAnyHashSessionIdValid()
{
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')));
}
/**
* Test is_session_id_valid with invalid IDs.
*/
public function testIsSessionIdInvalid()
{
$this->assertFalse(is_session_id_valid(''));
$this->assertFalse(is_session_id_valid(array()));
$this->assertFalse(
is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
);
}
/**
* Test generateSecretApi.
*/