diff options
-rw-r--r-- | application/SessionManager.php | 30 | ||||
-rw-r--r-- | application/Utils.php | 30 | ||||
-rw-r--r-- | index.php | 2 | ||||
-rw-r--r-- | tests/SessionManagerTest.php | 67 | ||||
-rw-r--r-- | tests/UtilsTest.php | 58 |
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 | */ | ||
197 | function 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 | * |
@@ -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. |
119 | if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) { | 119 | if (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 |
2 | namespace Shaarli; | 2 | // Initialize reference data _before_ PHPUnit starts a session |
3 | require_once 'tests/utils/ReferenceSessionIdHashes.php'; | ||
4 | ReferenceSessionIdHashes::genAllHashes(); | ||
3 | 5 | ||
6 | use \Shaarli\SessionManager; | ||
4 | use \PHPUnit\Framework\TestCase; | 7 | use \PHPUnit\Framework\TestCase; |
5 | 8 | ||
9 | |||
6 | /** | 10 | /** |
7 | * Fake ConfigManager | 11 | * Fake ConfigManager |
8 | */ | 12 | */ |
@@ -20,6 +24,17 @@ class FakeConfigManager | |||
20 | */ | 24 | */ |
21 | class SessionManagerTest extends TestCase | 25 | class 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 | ||
6 | require_once 'application/Utils.php'; | 6 | require_once 'application/Utils.php'; |
7 | require_once 'application/Languages.php'; | 7 | require_once 'application/Languages.php'; |
8 | require_once 'tests/utils/ReferenceSessionIdHashes.php'; | ||
9 | |||
10 | // Initialize reference data before PHPUnit starts a session | ||
11 | ReferenceSessionIdHashes::genAllHashes(); | ||
12 | 8 | ||
13 | 9 | ||
14 | /** | 10 | /** |
@@ -16,9 +12,6 @@ ReferenceSessionIdHashes::genAllHashes(); | |||
16 | */ | 12 | */ |
17 | class UtilsTest extends PHPUnit_Framework_TestCase | 13 | class 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() |