Session: SHA1-hash the session ID to ensure its format

Improves shaarli#306 Relates to shaarli#335 & shaarli#336 Issue: - the format of the value returned by `uniqid()` depends on PHP settings - the regex checking the session ID does not cover all cases Fix: - apply a hash function to the session ID (SHA1) See: - http://php.net/manual/en/session.configuration.php#ini.session.hash-function Signed-off-by: VirtualTam <[email protected]>
virtualtam · Sep 3, 2015 · 0c7481b · 0c7481b
1 parent ce8c4a8
commit 0c7481b
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 5 deletions.
diff --git a/application/Utils.php b/application/Utils.php
@@ -142,7 +142,7 @@ function checkPHPVersion($minVersion, $curVersion)
  * Validate session ID to prevent Full Path Disclosure.
  * See #298.
  *
- * @param string $sessionId Session ID
+ * @param string $sessionId Session ID (SHA1 hash)
  *
  * @return true if valid, false otherwise.
  */
@@ -156,7 +156,7 @@ function is_session_id_valid($sessionId)
         return false;
     }
 
-    if (!preg_match('/^[a-z0-9]{2,32}$/i', $sessionId)) {
+    if (!preg_match('/^[a-f0-9]{40}$/i', $sessionId)) {
         return false;
     }
 

diff --git a/index.php b/index.php
@@ -94,8 +94,9 @@
 
 // Regenerate session id if invalid or not defined in cookie.
 if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) {
-    $_COOKIE['shaarli'] = uniqid();
+    $_COOKIE['shaarli'] = sha1(uniqid());
 }
+
 session_name('shaarli');
 // Start session if needed (Some server auto-start sessions).
 if (session_id() == '') {

diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
@@ -156,7 +156,11 @@ public function testCheckSupportedPHPVersion52()
      */
     public function testIsSessionIdValid()
     {
-        $this->assertTrue(is_session_id_valid('azertyuiop123456789AZERTYUIOP1aA'));
+        $this->assertTrue(is_session_id_valid(sha1('shaarli')));
+        $this->assertTrue(is_session_id_valid(sha1(uniqid())));
+        $this->assertTrue(
+            is_session_id_valid('3908ac58488006753d29323fcbcf095b156dc177')
+        );
     }
 
     /**
@@ -166,6 +170,11 @@ 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=')
+        );
+        $this->assertFalse(
+            is_session_id_valid('zg3a58a1773c8e79e2211737d859231224ad6efe')
+        );
     }
 }