aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorVirtualTam <virtualtam@flibidi.net>2017-10-22 19:54:44 +0200
committerVirtualTam <virtualtam@flibidi.net>2017-10-22 19:54:44 +0200
commitfd7d84616d53486c3a276a42da869390e1d7f5eb (patch)
tree215f22ad244d734d77c3dd4a38f52da689fa6dd7
parentebd650c06c67a67da2a0d099f625b6a7ec62ab2b (diff)
downloadShaarli-fd7d84616d53486c3a276a42da869390e1d7f5eb.tar.gz
Shaarli-fd7d84616d53486c3a276a42da869390e1d7f5eb.tar.zst
Shaarli-fd7d84616d53486c3a276a42da869390e1d7f5eb.zip
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>
-rw-r--r--application/SessionManager.php30
-rw-r--r--application/Utils.php30
-rw-r--r--index.php2
-rw-r--r--tests/SessionManagerTest.php67
-rw-r--r--tests/UtilsTest.php58
5 files changed, 97 insertions, 90 deletions
diff --git a/application/SessionManager.php b/application/SessionManager.php
index 2083df42..3aa4ddfc 100644
--- a/application/SessionManager.php
+++ b/application/SessionManager.php
@@ -50,4 +50,34 @@ class SessionManager
50 unset($this->session['tokens'][$token]); 50 unset($this->session['tokens'][$token]);
51 return true; 51 return true;
52 } 52 }
53
54 /**
55 * Validate session ID to prevent Full Path Disclosure.
56 *
57 * See #298.
58 * The session ID's format depends on the hash algorithm set in PHP settings
59 *
60 * @param string $sessionId Session ID
61 *
62 * @return true if valid, false otherwise.
63 *
64 * @see http://php.net/manual/en/function.hash-algos.php
65 * @see http://php.net/manual/en/session.configuration.php
66 */
67 public static function checkId($sessionId)
68 {
69 if (empty($sessionId)) {
70 return false;
71 }
72
73 if (!$sessionId) {
74 return false;
75 }
76
77 if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
78 return false;
79 }
80
81 return true;
82 }
53} 83}
diff --git a/application/Utils.php b/application/Utils.php
index 2f38a8de..97b12fcf 100644
--- a/application/Utils.php
+++ b/application/Utils.php
@@ -182,36 +182,6 @@ function generateLocation($referer, $host, $loopTerms = array())
182} 182}
183 183
184/** 184/**
185 * Validate session ID to prevent Full Path Disclosure.
186 *
187 * See #298.
188 * The session ID's format depends on the hash algorithm set in PHP settings
189 *
190 * @param string $sessionId Session ID
191 *
192 * @return true if valid, false otherwise.
193 *
194 * @see http://php.net/manual/en/function.hash-algos.php
195 * @see http://php.net/manual/en/session.configuration.php
196 */
197function is_session_id_valid($sessionId)
198{
199 if (empty($sessionId)) {
200 return false;
201 }
202
203 if (!$sessionId) {
204 return false;
205 }
206
207 if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
208 return false;
209 }
210
211 return true;
212}
213
214/**
215 * Sniff browser language to set the locale automatically. 185 * Sniff browser language to set the locale automatically.
216 * Note that is may not work on your server if the corresponding locale is not installed. 186 * Note that is may not work on your server if the corresponding locale is not installed.
217 * 187 *
diff --git a/index.php b/index.php
index 9e698628..e1516d37 100644
--- a/index.php
+++ b/index.php
@@ -116,7 +116,7 @@ if (session_id() == '') {
116} 116}
117 117
118// Regenerate session ID if invalid or not defined in cookie. 118// Regenerate session ID if invalid or not defined in cookie.
119if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) { 119if (isset($_COOKIE['shaarli']) && !SessionManager::checkId($_COOKIE['shaarli'])) {
120 session_regenerate_id(true); 120 session_regenerate_id(true);
121 $_COOKIE['shaarli'] = session_id(); 121 $_COOKIE['shaarli'] = session_id();
122} 122}
diff --git a/tests/SessionManagerTest.php b/tests/SessionManagerTest.php
index 3a270303..9fa60dc5 100644
--- a/tests/SessionManagerTest.php
+++ b/tests/SessionManagerTest.php
@@ -1,8 +1,12 @@
1<?php 1<?php
2namespace Shaarli; 2// Initialize reference data _before_ PHPUnit starts a session
3require_once 'tests/utils/ReferenceSessionIdHashes.php';
4ReferenceSessionIdHashes::genAllHashes();
3 5
6use \Shaarli\SessionManager;
4use \PHPUnit\Framework\TestCase; 7use \PHPUnit\Framework\TestCase;
5 8
9
6/** 10/**
7 * Fake ConfigManager 11 * Fake ConfigManager
8 */ 12 */
@@ -20,6 +24,17 @@ class FakeConfigManager
20 */ 24 */
21class SessionManagerTest extends TestCase 25class SessionManagerTest extends TestCase
22{ 26{
27 // Session ID hashes
28 protected static $sidHashes = null;
29
30 /**
31 * Assign reference data
32 */
33 public static function setUpBeforeClass()
34 {
35 self::$sidHashes = ReferenceSessionIdHashes::getHashes();
36 }
37
23 /** 38 /**
24 * Generate a session token 39 * Generate a session token
25 */ 40 */
@@ -69,4 +84,54 @@ class SessionManagerTest extends TestCase
69 84
70 $this->assertFalse($sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b')); 85 $this->assertFalse($sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
71 } 86 }
87
88 /**
89 * Test SessionManager::checkId with a valid ID - TEST ALL THE HASHES!
90 *
91 * This tests extensively covers all hash algorithms / bit representations
92 */
93 public function testIsAnyHashSessionIdValid()
94 {
95 foreach (self::$sidHashes as $algo => $bpcs) {
96 foreach ($bpcs as $bpc => $hash) {
97 $this->assertTrue(SessionManager::checkId($hash));
98 }
99 }
100 }
101
102 /**
103 * Test checkId with a valid ID - SHA-1 hashes
104 */
105 public function testIsSha1SessionIdValid()
106 {
107 $this->assertTrue(SessionManager::checkId(sha1('shaarli')));
108 }
109
110 /**
111 * Test checkId with a valid ID - SHA-256 hashes
112 */
113 public function testIsSha256SessionIdValid()
114 {
115 $this->assertTrue(SessionManager::checkId(hash('sha256', 'shaarli')));
116 }
117
118 /**
119 * Test checkId with a valid ID - SHA-512 hashes
120 */
121 public function testIsSha512SessionIdValid()
122 {
123 $this->assertTrue(SessionManager::checkId(hash('sha512', 'shaarli')));
124 }
125
126 /**
127 * Test checkId with invalid IDs.
128 */
129 public function testIsSessionIdInvalid()
130 {
131 $this->assertFalse(SessionManager::checkId(''));
132 $this->assertFalse(SessionManager::checkId([]));
133 $this->assertFalse(
134 SessionManager::checkId('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
135 );
136 }
72} 137}
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
index 840eaf21..6cd37a7a 100644
--- a/tests/UtilsTest.php
+++ b/tests/UtilsTest.php
@@ -5,10 +5,6 @@
5 5
6require_once 'application/Utils.php'; 6require_once 'application/Utils.php';
7require_once 'application/Languages.php'; 7require_once 'application/Languages.php';
8require_once 'tests/utils/ReferenceSessionIdHashes.php';
9
10// Initialize reference data before PHPUnit starts a session
11ReferenceSessionIdHashes::genAllHashes();
12 8
13 9
14/** 10/**
@@ -16,9 +12,6 @@ ReferenceSessionIdHashes::genAllHashes();
16 */ 12 */
17class UtilsTest extends PHPUnit_Framework_TestCase 13class UtilsTest extends PHPUnit_Framework_TestCase
18{ 14{
19 // Session ID hashes
20 protected static $sidHashes = null;
21
22 // Log file 15 // Log file
23 protected static $testLogFile = 'tests.log'; 16 protected static $testLogFile = 'tests.log';
24 17
@@ -30,13 +23,11 @@ class UtilsTest extends PHPUnit_Framework_TestCase
30 */ 23 */
31 protected static $defaultTimeZone; 24 protected static $defaultTimeZone;
32 25
33
34 /** 26 /**
35 * Assign reference data 27 * Assign reference data
36 */ 28 */
37 public static function setUpBeforeClass() 29 public static function setUpBeforeClass()
38 { 30 {
39 self::$sidHashes = ReferenceSessionIdHashes::getHashes();
40 self::$defaultTimeZone = date_default_timezone_get(); 31 self::$defaultTimeZone = date_default_timezone_get();
41 // Timezone without DST for test consistency 32 // Timezone without DST for test consistency
42 date_default_timezone_set('Africa/Nairobi'); 33 date_default_timezone_set('Africa/Nairobi');
@@ -221,57 +212,8 @@ class UtilsTest extends PHPUnit_Framework_TestCase
221 $this->assertEquals('?', generateLocation($ref, 'localhost')); 212 $this->assertEquals('?', generateLocation($ref, 'localhost'));
222 } 213 }
223 214
224 /**
225 * Test is_session_id_valid with a valid ID - TEST ALL THE HASHES!
226 *
227 * This tests extensively covers all hash algorithms / bit representations
228 */
229 public function testIsAnyHashSessionIdValid()
230 {
231 foreach (self::$sidHashes as $algo => $bpcs) {
232 foreach ($bpcs as $bpc => $hash) {
233 $this->assertTrue(is_session_id_valid($hash));
234 }
235 }
236 }
237 215
238 /** 216 /**
239 * Test is_session_id_valid with a valid ID - SHA-1 hashes
240 */
241 public function testIsSha1SessionIdValid()
242 {
243 $this->assertTrue(is_session_id_valid(sha1('shaarli')));
244 }
245
246 /**
247 * Test is_session_id_valid with a valid ID - SHA-256 hashes
248 */
249 public function testIsSha256SessionIdValid()
250 {
251 $this->assertTrue(is_session_id_valid(hash('sha256', 'shaarli')));
252 }
253
254 /**
255 * Test is_session_id_valid with a valid ID - SHA-512 hashes
256 */
257 public function testIsSha512SessionIdValid()
258 {
259 $this->assertTrue(is_session_id_valid(hash('sha512', 'shaarli')));
260 }
261
262 /**
263 * Test is_session_id_valid with invalid IDs.
264 */
265 public function testIsSessionIdInvalid()
266 {
267 $this->assertFalse(is_session_id_valid(''));
268 $this->assertFalse(is_session_id_valid(array()));
269 $this->assertFalse(
270 is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
271 );
272 }
273
274 /**
275 * Test generateSecretApi. 217 * Test generateSecretApi.
276 */ 218 */
277 public function testGenerateSecretApi() 219 public function testGenerateSecretApi()