diff options
author | VirtualTam <virtualtam@flibidi.net> | 2015-09-03 23:12:58 +0200 |
---|---|---|
committer | VirtualTam <virtualtam@flibidi.net> | 2015-09-06 16:14:24 +0200 |
commit | 68bc21353a6138a898724c8bb87684bb2b6b2c1c (patch) | |
tree | 8c100e6ca4cba5870640cf3e0ec688b1f0fa7474 | |
parent | a02257b8aed58ef2f8536c877ce2fb222f84ac40 (diff) | |
download | Shaarli-68bc21353a6138a898724c8bb87684bb2b6b2c1c.tar.gz Shaarli-68bc21353a6138a898724c8bb87684bb2b6b2c1c.tar.zst Shaarli-68bc21353a6138a898724c8bb87684bb2b6b2c1c.zip |
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>
-rwxr-xr-x | application/Utils.php | 7 | ||||
-rwxr-xr-x | index.php | 10 | ||||
-rwxr-xr-x | tests/UtilsTest.php | 56 | ||||
-rw-r--r-- | tests/utils/ReferenceSessionIdHashes.php | 55 |
4 files changed, 119 insertions, 9 deletions
diff --git a/application/Utils.php b/application/Utils.php index cb03f11c..1422961d 100755 --- a/application/Utils.php +++ b/application/Utils.php | |||
@@ -140,11 +140,16 @@ function checkPHPVersion($minVersion, $curVersion) | |||
140 | 140 | ||
141 | /** | 141 | /** |
142 | * Validate session ID to prevent Full Path Disclosure. | 142 | * Validate session ID to prevent Full Path Disclosure. |
143 | * | ||
143 | * See #298. | 144 | * See #298. |
145 | * The session ID's format depends on the hash algorithm set in PHP settings | ||
144 | * | 146 | * |
145 | * @param string $sessionId Session ID | 147 | * @param string $sessionId Session ID |
146 | * | 148 | * |
147 | * @return true if valid, false otherwise. | 149 | * @return true if valid, false otherwise. |
150 | * | ||
151 | * @see http://php.net/manual/en/function.hash-algos.php | ||
152 | * @see http://php.net/manual/en/session.configuration.php | ||
148 | */ | 153 | */ |
149 | function is_session_id_valid($sessionId) | 154 | function is_session_id_valid($sessionId) |
150 | { | 155 | { |
@@ -156,7 +161,7 @@ function is_session_id_valid($sessionId) | |||
156 | return false; | 161 | return false; |
157 | } | 162 | } |
158 | 163 | ||
159 | if (!preg_match('/^[a-z0-9]{2,32}$/i', $sessionId)) { | 164 | if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) { |
160 | return false; | 165 | return false; |
161 | } | 166 | } |
162 | 167 | ||
@@ -92,16 +92,18 @@ ini_set('session.use_only_cookies', 1); | |||
92 | // Prevent PHP form using sessionID in URL if cookies are disabled. | 92 | // Prevent PHP form using sessionID in URL if cookies are disabled. |
93 | ini_set('session.use_trans_sid', false); | 93 | ini_set('session.use_trans_sid', false); |
94 | 94 | ||
95 | // Regenerate session id if invalid or not defined in cookie. | ||
96 | if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) { | ||
97 | $_COOKIE['shaarli'] = uniqid(); | ||
98 | } | ||
99 | session_name('shaarli'); | 95 | session_name('shaarli'); |
100 | // Start session if needed (Some server auto-start sessions). | 96 | // Start session if needed (Some server auto-start sessions). |
101 | if (session_id() == '') { | 97 | if (session_id() == '') { |
102 | session_start(); | 98 | session_start(); |
103 | } | 99 | } |
104 | 100 | ||
101 | // Regenerate session ID if invalid or not defined in cookie. | ||
102 | if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) { | ||
103 | session_regenerate_id(true); | ||
104 | $_COOKIE['shaarli'] = session_id(); | ||
105 | } | ||
106 | |||
105 | include "inc/rain.tpl.class.php"; //include Rain TPL | 107 | include "inc/rain.tpl.class.php"; //include Rain TPL |
106 | raintpl::$tpl_dir = $GLOBALS['config']['RAINTPL_TPL']; // template directory | 108 | raintpl::$tpl_dir = $GLOBALS['config']['RAINTPL_TPL']; // template directory |
107 | raintpl::$cache_dir = $GLOBALS['config']['RAINTPL_TMP']; // cache directory | 109 | raintpl::$cache_dir = $GLOBALS['config']['RAINTPL_TMP']; // cache directory |
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 5175dde0..7f218ad5 100755 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php | |||
@@ -4,12 +4,28 @@ | |||
4 | */ | 4 | */ |
5 | 5 | ||
6 | require_once 'application/Utils.php'; | 6 | require_once 'application/Utils.php'; |
7 | require_once 'tests/utils/ReferenceSessionIdHashes.php'; | ||
8 | |||
9 | // Initialize reference data before PHPUnit starts a session | ||
10 | ReferenceSessionIdHashes::genAllHashes(); | ||
11 | |||
7 | 12 | ||
8 | /** | 13 | /** |
9 | * Unitary tests for Shaarli utilities | 14 | * Unitary tests for Shaarli utilities |
10 | */ | 15 | */ |
11 | class UtilsTest extends PHPUnit_Framework_TestCase | 16 | class UtilsTest extends PHPUnit_Framework_TestCase |
12 | { | 17 | { |
18 | // Session ID hashes | ||
19 | protected static $sidHashes = null; | ||
20 | |||
21 | /** | ||
22 | * Assign reference data | ||
23 | */ | ||
24 | public static function setUpBeforeClass() | ||
25 | { | ||
26 | self::$sidHashes = ReferenceSessionIdHashes::getHashes(); | ||
27 | } | ||
28 | |||
13 | /** | 29 | /** |
14 | * Represent a link by its hash | 30 | * Represent a link by its hash |
15 | */ | 31 | */ |
@@ -152,11 +168,41 @@ class UtilsTest extends PHPUnit_Framework_TestCase | |||
152 | } | 168 | } |
153 | 169 | ||
154 | /** | 170 | /** |
155 | * Test is_session_id_valid with a valid ID. | 171 | * Test is_session_id_valid with a valid ID - TEST ALL THE HASHES! |
172 | * | ||
173 | * This tests extensively covers all hash algorithms / bit representations | ||
174 | */ | ||
175 | public function testIsAnyHashSessionIdValid() | ||
176 | { | ||
177 | foreach (self::$sidHashes as $algo => $bpcs) { | ||
178 | foreach ($bpcs as $bpc => $hash) { | ||
179 | $this->assertTrue(is_session_id_valid($hash)); | ||
180 | } | ||
181 | } | ||
182 | } | ||
183 | |||
184 | /** | ||
185 | * Test is_session_id_valid with a valid ID - SHA-1 hashes | ||
186 | */ | ||
187 | public function testIsSha1SessionIdValid() | ||
188 | { | ||
189 | $this->assertTrue(is_session_id_valid(sha1('shaarli'))); | ||
190 | } | ||
191 | |||
192 | /** | ||
193 | * Test is_session_id_valid with a valid ID - SHA-256 hashes | ||
194 | */ | ||
195 | public function testIsSha256SessionIdValid() | ||
196 | { | ||
197 | $this->assertTrue(is_session_id_valid(hash('sha256', 'shaarli'))); | ||
198 | } | ||
199 | |||
200 | /** | ||
201 | * Test is_session_id_valid with a valid ID - SHA-512 hashes | ||
156 | */ | 202 | */ |
157 | public function testIsSessionIdValid() | 203 | public function testIsSha512SessionIdValid() |
158 | { | 204 | { |
159 | $this->assertTrue(is_session_id_valid('azertyuiop123456789AZERTYUIOP1aA')); | 205 | $this->assertTrue(is_session_id_valid(hash('sha512', 'shaarli'))); |
160 | } | 206 | } |
161 | 207 | ||
162 | /** | 208 | /** |
@@ -166,6 +212,8 @@ class UtilsTest extends PHPUnit_Framework_TestCase | |||
166 | { | 212 | { |
167 | $this->assertFalse(is_session_id_valid('')); | 213 | $this->assertFalse(is_session_id_valid('')); |
168 | $this->assertFalse(is_session_id_valid(array())); | 214 | $this->assertFalse(is_session_id_valid(array())); |
169 | $this->assertFalse(is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')); | 215 | $this->assertFalse( |
216 | is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=') | ||
217 | ); | ||
170 | } | 218 | } |
171 | } | 219 | } |
diff --git a/tests/utils/ReferenceSessionIdHashes.php b/tests/utils/ReferenceSessionIdHashes.php new file mode 100644 index 00000000..60b1c007 --- /dev/null +++ b/tests/utils/ReferenceSessionIdHashes.php | |||
@@ -0,0 +1,55 @@ | |||
1 | <?php | ||
2 | /** | ||
3 | * Testing the untestable - Session ID generation | ||
4 | */ | ||
5 | class ReferenceSessionIdHashes | ||
6 | { | ||
7 | // Session ID hashes | ||
8 | protected static $sidHashes = null; | ||
9 | |||
10 | /** | ||
11 | * Generates session ID hashes for all algorithms & bit representations | ||
12 | */ | ||
13 | public static function genAllHashes() | ||
14 | { | ||
15 | foreach (hash_algos() as $algo) { | ||
16 | self::$sidHashes[$algo] = array(); | ||
17 | |||
18 | foreach (array(4, 5, 6) as $bpc) { | ||
19 | self::$sidHashes[$algo][$bpc] = self::genSidHash($algo, $bpc); | ||
20 | } | ||
21 | } | ||
22 | } | ||
23 | |||
24 | /** | ||
25 | * Generates a session ID for a given hash algorithm and bit representation | ||
26 | * | ||
27 | * @param string $function name of the hash function | ||
28 | * @param int $bits_per_character representation type | ||
29 | * | ||
30 | * @return string the generated session ID | ||
31 | */ | ||
32 | protected static function genSidHash($function, $bits_per_character) | ||
33 | { | ||
34 | if (session_id()) { | ||
35 | session_destroy(); | ||
36 | } | ||
37 | |||
38 | ini_set('session.hash_function', $function); | ||
39 | ini_set('session.hash_bits_per_character', $bits_per_character); | ||
40 | |||
41 | session_start(); | ||
42 | return session_id(); | ||
43 | } | ||
44 | |||
45 | /** | ||
46 | * Returns the reference hash array | ||
47 | * | ||
48 | * @return array session IDs generated for all available algorithms and bit | ||
49 | * representations | ||
50 | */ | ||
51 | public static function getHashes() | ||
52 | { | ||
53 | return self::$sidHashes; | ||
54 | } | ||
55 | } | ||