validate session id in File handler before doing file I/O (#2)

maintaina-com · Aug 23, 2024 · 58f248d · 58f248d
1 parent fefa7c3
commit 58f248d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 4 deletions.
diff --git a/lib/Horde/SessionHandler/Storage.php b/lib/Horde/SessionHandler/Storage.php
@@ -131,4 +131,15 @@ abstract public function gc($maxlifetime = 300);
      */
     abstract public function getSessionIDs();
 
+    /**
+     * Validate a session id against the schema mentioned in the PHP manual page for session_id()
+     *
+     * @param string $id
+     * @return bool
+     */
+    protected function isValidSessionID(string $id): bool
+    {
+       return (preg_match('/^[A-Za-z0-9,-]{22,256}$/', $id) === 1);
+    }
+
 }
diff --git a/lib/Horde/SessionHandler/Storage/File.php b/lib/Horde/SessionHandler/Storage/File.php
@@ -64,10 +64,12 @@ public function open($save_path = null, $session_name = null)
      *
      * @param string $id  The session ID.
      */
-    protected function _open($id)
+    protected function _open(string $id)
     {
         if (!empty($this->_fp)) {
             return;
+        } elseif (!$this->isValidSessionID($id)) {
+            return;
         }
 
         $filename = $this->_params['path'] . '/' . self::PREFIX . $id;
@@ -79,8 +81,9 @@ protected function _open($id)
     }
 
     /**
+     * @return bool
      */
-    public function close()
+    public function close(): bool
     {
         if (!empty($this->_fp)) {
             flock($this->_fp, LOCK_UN);
@@ -113,7 +116,7 @@ public function read($id)
 
     /**
      */
-    public function write($id, $session_data)
+    public function write($id, $session_data): bool
     {
         $this->_open($id);
 
@@ -129,11 +132,17 @@ public function write($id, $session_data)
     }
 
     /**
+     * @param string $id
+     * @return bool
      */
-    public function destroy($id)
+    public function destroy($id): bool
     {
         $this->close();
 
+        if (!$this->isValidSessionID($id)) {
+            return false;
+        }
+
         $filename = $this->_params['path'] . '/' . self::PREFIX . $id;
 
         return @unlink($filename);