Fix ZF-3378: Zend_Session allow invalid session ids. Patch by Adam Lundrigan

git-svn-id: http://framework.zend.com/svn/framework/standard/trunk@24819 44c647ce-9c0f-0410-b52a-842ac1e357ba

library/Zend/Session.php

@@ -415,6 +415,14 @@ class Zend_Session extends Zend_Session_Abstract
      */
     public static function start($options = false)
     {
+        // Check to see if we've been passed an invalid session ID
+        if ( self::getId() && !self::_checkId(self::getId()) ) {
+            // Generate a valid, temporary replacement
+            self::setId(md5(self::getId()));
+            // Force a regenerate after session is started
+            self::$_regenerateIdState = -1;
+        }
+
         if (self::$_sessionStarted && self::$_destroyed) {
             require_once 'Zend/Session/Exception.php';
             throw new Zend_Session_Exception('The session was explicitly destroyed during this request, attempting to re-start is not allowed.');
@@ -499,6 +507,26 @@ class Zend_Session extends Zend_Session_Abstract
         self::_processStartupMetadataGlobal();
     }
 
+    /**
+     * Perform a hash-bits check on the session ID
+     *
+     * @param string $id Session ID
+     * @return bool
+     */
+    protected static function _checkId($id)
+    {
+        $hashBitsPerChar = ini_get('session.hash_bits_per_character');
+        if (!$hashBitsPerChar) {
+            $hashBitsPerChar = 5; // the default value
+        }
+        switch($hashBitsPerChar) {
+            case 4: $pattern = '^[0-9a-f]*$'; break;
+            case 5: $pattern = '^[0-9a-v]*$'; break;
+            case 6: $pattern = '^[0-9a-zA-Z-,]*$'; break;
+        }
+        return preg_match('#'.$pattern.'#', $id);
+    }
+
 
     /**
      * _processGlobalMetadata() - this method initizes the sessions GLOBAL

tests/Zend/Session/SessionTest.php

@@ -1063,4 +1063,51 @@ class Zend_SessionTest extends PHPUnit_Framework_TestCase
         }
     }
 
+    /**
+     * @group ZF-3378
+     */
+    public function testInvalidPreexistingSessionIdDoesNotPreventRegenerationOfSid()
+    {
+        // Pattern: [0-9a-v]*
+        ini_set('session.hash_bits_per_character', 5);
+
+        // Session store
+        $sessionCharSet = array_merge(range(0,9), range('a','v'));
+        $sessionStore = dirname(__FILE__)
+                      . DIRECTORY_SEPARATOR . "_files"
+                      . DIRECTORY_SEPARATOR . "ZF-3378";
+        if ( !is_dir($sessionStore) ) @mkdir($sessionStore, 0755, true);
+        ini_set('session.save_path', "1;666;" . $sessionStore);
+
+        // When using subdirs for session.save_path, the directory structure
+        // is your own responsibility...set it up, or else bad things happen
+        foreach ( $sessionCharSet as $subdir ) {
+            @mkdir($sessionStore . DIRECTORY_SEPARATOR . $subdir);
+        }
+
+        // Set session ID to invalid value
+        session_id('xxx');
+
+        // Attempt to start the session
+        try {
+            /** @see Zend_Session */
+            require_once "Zend/Session.php";
+            Zend_Session::start();
+        } catch (Zend_Session_Exception $e) {
+            Zend_Session::regenerateId();
+        }
+        // Get the current SID
+        $sid = Zend_Session::getId();
+
+        // We don't need the session any more, clean it up
+        Zend_Session::destroy();
+        foreach ( $sessionCharSet as $subdir ) {
+            @rmdir($sessionStore . DIRECTORY_SEPARATOR . $subdir);
+        }
+        @rmdir($sessionStore);
+
+        // Check the result
+        $this->assertRegExp('/^[0-9a-v]+$/', $sid);
+        $this->assertNotEquals('xxx', $sid);
+    }
 }