Fix #18247: Added support for the 'session.use_strict_mode' ini directive in yii\web\Session

.github/workflows/build.yml

@@ -56,7 +56,7 @@ jobs:
           php-version: ${{ matrix.php }}
           tools: pecl
           extensions: apc, curl, dom, imagick, intl, mbstring, mcrypt, memcached, mysql, pdo, pdo_mysql, pdo_pgsql, pdo_sqlite, pgsql, sqlite
-          ini-values: date.timezone='UTC'
+          ini-values: date.timezone='UTC', session.save_path="${{ runner.temp }}"
       - name: Install php-sqlsrv
         run: |
             curl -sSL https://packages.microsoft.com/keys/microsoft.asc | sudo apt-key add - >/dev/null 2>&1
@@ -114,6 +114,7 @@ jobs:
         uses: shivammathur/setup-php@v2
         with:
           php-version: 7.2
+          ini-values: session.save_path=${{ runner.temp }}
       - name: Get composer cache directory
         id: composer-cache
         run: echo "::set-output name=dir::$(composer config cache-files-dir)"

docs/guide/runtime-sessions-cookies.md

@@ -403,3 +403,4 @@ To use this feature across different PHP versions check the version first. E.g.
 As [noted in PHP manual](https://www.php.net/manual/en/session.security.ini.php), `php.ini` has important
 session security settings. Please ensure recommended settings are applied. Especially `session.use_strict_mode`
 that is not enabled by default in PHP installations.
+This setting can also be set with [[yii\web\Session::useStrictMode]].

framework/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
 2.0.39 under development
 ------------------------
 
+- Enh #18247: Added support for the 'session.use_strict_mode' ini directive in `yii\web\Session` (rhertogh)
 - Bug #18263: Fix writing `\yii\caching\FileCache` files to the same directory when `keyPrefix` is set (githubjeka)
 - Bug #18160, #18192: Fixed `registerFile` with argument depends set does not use the position and appendTimestamp argument, also modify the unit view (baleeny)
 - Bug #18290: Fix response with non-seekable streams (schmunk42)

framework/UPGRADE.md

@@ -63,6 +63,41 @@ Upgrade from Yii 2.0.37
 * Resolving DI references inside of arrays in dependencies was made optional and turned off by default. In order
   to turn it on, set `resolveArrays` of container instance to `true`.
 
+* `yii\web\Session` now respects the 'session.use_strict_mode' ini directive.
+  In case you use a custom `Session` class and have overwritten the `Session::openSession()` and/or 
+  `Session::writeSession()` functions changes might be required:
+  * When in strict mode the `openSession()` function should check if the requested session id exists
+    (and mark it for forced regeneration if not).
+    For example, the `DbSession` does this at the beginning of the function as follows:
+    ```php
+    if ($this->getUseStrictMode()) {
+        $id = $this->getId();
+        if (!$this->getReadQuery($id)->exists()) {
+            //This session id does not exist, mark it for forced regeneration
+            $this->_forceRegenerateId = $id;
+        }
+    }
+    // ... normal function continues ...
+    ``` 
+  * When in strict mode the `writeSession()` function should ignore writing the session under the old id.
+    For example, the `DbSession` does this at the beginning of the function as follows:
+    ```php
+    if ($this->getUseStrictMode() && $id === $this->_forceRegenerateId) {
+        //Ignore write when forceRegenerate is active for this id
+        return true;
+    }
+    // ... normal function continues ...
+    ```
+  > Note: The sample code above is specific for the `yii\web\DbSession` class.
+    Make sure you use the correct implementation based on your parent class,
+    e.g. `yii\web\CacheSession`, `yii\redis\Session`, `yii\mongodb\Session`, etc.
+  
+  > Note: In case your custom functions call their `parent` functions, there are probably no changes needed to your 
+    code if those parents implement the `useStrictMode` checks.
+
+  > Warning: in case `openSession()` and/or `writeSession()` functions do not implement the `useStrictMode` code
+    the session could be stored under the forced id without warning even if `useStrictMode` is enabled.
+
 Upgrade from Yii 2.0.36
 -----------------------
 

framework/web/CacheSession.php

@@ -69,6 +69,26 @@ class CacheSession extends Session
         return true;
     }
 
+    /**
+     * Session open handler.
+     * @internal Do not call this method directly.
+     * @param string $savePath session save path
+     * @param string $sessionName session name
+     * @return bool whether session is opened successfully
+     */
+    public function openSession($savePath, $sessionName)
+    {
+        if ($this->getUseStrictMode()) {
+            $id = $this->getId();
+            if (!$this->cache->exists($this->calculateKey($id))) {
+                //This session id does not exist, mark it for forced regeneration
+                $this->_forceRegenerateId = $id;
+            }
+        }
+
+        return parent::openSession($savePath, $sessionName);
+    }
+
     /**
      * Session read handler.
      * @internal Do not call this method directly.
@@ -91,6 +111,11 @@ class CacheSession extends Session
      */
     public function writeSession($id, $data)
     {
+        if ($this->getUseStrictMode() && $id === $this->_forceRegenerateId) {
+            //Ignore write when forceRegenerate is active for this id
+            return true;
+        }
+
         return $this->cache->set($this->calculateKey($id), $data, $this->getTimeout());
     }
 

framework/web/DbSession.php

@@ -93,6 +93,26 @@ class DbSession extends MultiFieldSession
         $this->db = Instance::ensure($this->db, Connection::className());
     }
 
+    /**
+     * Session open handler.
+     * @internal Do not call this method directly.
+     * @param string $savePath session save path
+     * @param string $sessionName session name
+     * @return bool whether session is opened successfully
+     */
+    public function openSession($savePath, $sessionName)
+    {
+        if ($this->getUseStrictMode()) {
+            $id = $this->getId();
+            if (!$this->getReadQuery($id)->exists()) {
+                //This session id does not exist, mark it for forced regeneration
+                $this->_forceRegenerateId = $id;
+            }
+        }
+
+        return parent::openSession($savePath, $sessionName);
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -160,9 +180,7 @@ class DbSession extends MultiFieldSession
      */
     public function readSession($id)
     {
-        $query = new Query();
-        $query->from($this->sessionTable)
-            ->where('[[expire]]>:expire AND [[id]]=:id', [':expire' => time(), ':id' => $id]);
+        $query = $this->getReadQuery($id);
 
         if ($this->readCallback !== null) {
             $fields = $query->one($this->db);
@@ -182,6 +200,11 @@ class DbSession extends MultiFieldSession
      */
     public function writeSession($id, $data)
     {
+        if ($this->getUseStrictMode() && $id === $this->_forceRegenerateId) {
+            //Ignore write when forceRegenerate is active for this id
+            return true;
+        }
+
         // exception must be caught in session write handler
         // https://secure.php.net/manual/en/function.session-set-save-handler.php#refsect1-function.session-set-save-handler-notes
         try {
@@ -240,6 +263,18 @@ class DbSession extends MultiFieldSession
         return true;
     }
 
+    /**
+     * Generates a query to get the session from db
+     * @param string $id The id of the session
+     * @return Query
+     */
+    protected function getReadQuery($id)
+    {
+        return (new Query())
+            ->from($this->sessionTable)
+            ->where('[[expire]]>:expire AND [[id]]=:id', [':expire' => time(), ':id' => $id]);
+    }
+
     /**
      * Method typecasts $fields before passing them to PDO.
      * Default implementation casts field `data` to `\PDO::PARAM_LOB`.

framework/web/Session.php

@@ -68,12 +68,25 @@ use yii\base\InvalidConfigException;
  * @property bool $useCustomStorage Whether to use custom storage. This property is read-only.
  * @property bool $useTransparentSessionID Whether transparent sid support is enabled or not, defaults to
  * false.
+ * @property bool $useStrictMode Whether strict mode is enabled or not.
+ * Note: Enabling `useStrictMode` on PHP < 5.5.2 is only supported with custom storage classes.
  *
  * @author Qiang Xue <qiang.xue@gmail.com>
  * @since 2.0
  */
 class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Countable
 {
+    /**
+     * @var string|null Holds the original session module (before a custom handler is registered) so that it can be
+     * restored when a Session component without custom handler is used after one that has.
+     */
+    static protected $_originalSessionModule = null;
+
+    /**
+     * Polyfill for ini directive session.use-strict-mode for PHP < 5.5.2.
+     */
+    static private $_useStrictModePolyfill = false;
+
     /**
      * @var string the name of the session variable that stores the flash message data.
      */
@@ -82,6 +95,10 @@ class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Co
      * @var \SessionHandlerInterface|array an object implementing the SessionHandlerInterface or a configuration array. If set, will be used to provide persistency instead of build-in methods.
      */
     public $handler;
+    /**
+     * @var string|null Holds the session id in case useStrictMode is enabled and the session id needs to be regenerated
+     */
+    protected $_forceRegenerateId = null;
 
     /**
      * @var array parameter-value pairs to override default session cookie parameters that are used for session_set_cookie_params() function
@@ -136,6 +153,11 @@ class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Co
 
         YII_DEBUG ? session_start() : @session_start();
 
+        if ($this->getUseStrictMode() && $this->_forceRegenerateId) {
+            $this->regenerateID();
+            $this->_forceRegenerateId = null;
+        }
+
         if ($this->getIsActive()) {
             Yii::info('Session started', __METHOD__);
             $this->updateFlashCounters();
@@ -152,6 +174,11 @@ class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Co
      */
     protected function registerSessionHandler()
     {
+        $sessionModuleName = session_module_name();
+        if (static::$_originalSessionModule === null) {
+            static::$_originalSessionModule = $sessionModuleName;
+        }
+
         if ($this->handler !== null) {
             if (!is_object($this->handler)) {
                 $this->handler = Yii::createObject($this->handler);
@@ -180,6 +207,12 @@ class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Co
                     [$this, 'gcSession']
                 );
             }
+        } elseif (
+            $sessionModuleName !== static::$_originalSessionModule
+            && static::$_originalSessionModule !== null
+            && static::$_originalSessionModule !== 'user'
+        ) {
+            session_module_name(static::$_originalSessionModule);
         }
     }
 
@@ -191,6 +224,8 @@ class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Co
         if ($this->getIsActive()) {
             YII_DEBUG ? session_write_close() : @session_write_close();
         }
+
+        $this->_forceRegenerateId = null;
     }
 
     /**
@@ -514,6 +549,43 @@ class Session extends Component implements \IteratorAggregate, \ArrayAccess, \Co
         $this->unfreeze();
     }
 
+    /**
+     * @var bool Whether strict mode is enabled or not.
+     * When `true` this setting prevents the session component to use an uninitialized session ID.
+     * Note: Enabling `useStrictMode` on PHP < 5.5.2 is only supported with custom storage classes.
+     * Warning! Although enabling strict mode is mandatory for secure sessions, the default value of 'session.use-strict-mode' is `0`.
+     * @see https://www.php.net/manual/en/session.configuration.php#ini.session.use-strict-mode
+     * @since 2.0.38
+     */
+    public function setUseStrictMode($value)
+    {
+        if (PHP_VERSION_ID < 50502) {
+            if ($this->getUseCustomStorage() || !$value) {
+                self::$_useStrictModePolyfill = $value;
+            } else {
+                throw new InvalidConfigException('Enabling `useStrictMode` on PHP < 5.5.2 is only supported with custom storage classes.');
+            }
+        } else {
+            $this->freeze();
+            ini_set('session.use_strict_mode', $value ? '1' : '0');
+            $this->unfreeze();
+        }
+    }
+
+    /**
+     * @return bool Whether strict mode is enabled or not.
+     * @see setUseStrictMode()
+     * @since 2.0.38
+     */
+    public function getUseStrictMode()
+    {
+        if (PHP_VERSION_ID < 50502) {
+            return self::$_useStrictModePolyfill;
+        }
+
+        return (bool)ini_get('session.use_strict_mode');
+    }
+
     /**
      * Session open handler.
      * This method should be overridden if [[useCustomStorage]] returns true.

tests/framework/web/session/AbstractDbSessionTest.php

@@ -9,8 +9,8 @@ namespace yiiunit\framework\web\session;
 
 use Yii;
 use yii\db\Connection;
-use yii\db\Query;
 use yii\db\Migration;
+use yii\db\Query;
 use yii\web\DbSession;
 use yiiunit\framework\console\controllers\EchoMigrateController;
 use yiiunit\TestCase;
@@ -20,6 +20,8 @@ use yiiunit\TestCase;
  */
 abstract class AbstractDbSessionTest extends TestCase
 {
+    use SessionTestTrait;
+
     /**
      * @return string[] the driver names that are suitable for the test (mysql, pgsql, etc)
      */
@@ -266,4 +268,14 @@ abstract class AbstractDbSessionTest extends TestCase
         Yii::$app->set('sessionDb', null);
         ini_set('session.gc_maxlifetime', $oldTimeout);
     }
+
+    public function testInitUseStrictMode()
+    {
+        $this->initStrictModeTest(DbSession::className());
+    }
+
+    public function testUseStrictMode()
+    {
+        $this->useStrictModeTest(DbSession::className());
+    }
 }

tests/framework/web/session/CacheSessionTest.php

@@ -16,6 +16,8 @@ use yii\web\CacheSession;
  */
 class CacheSessionTest extends \yiiunit\TestCase
 {
+    use SessionTestTrait;
+
     protected function setUp()
     {
         parent::setUp();
@@ -51,4 +53,14 @@ class CacheSessionTest extends \yiiunit\TestCase
 
         $this->assertTrue($session->destroySession($session->getId()));
     }
+
+    public function testInitUseStrictMode()
+    {
+        $this->initStrictModeTest(CacheSession::className());
+    }
+
+    public function testUseStrictMode()
+    {
+        $this->useStrictModeTest(CacheSession::className());
+    }
 }

tests/framework/web/session/SessionTest.php

@@ -15,6 +15,8 @@ use yiiunit\TestCase;
  */
 class SessionTest extends TestCase
 {
+    use SessionTestTrait;
+
     /**
      * Test to prove that after Session::destroy session id set to old value.
      */
@@ -69,6 +71,7 @@ class SessionTest extends TestCase
         $newGcProbability = $session->getGCProbability();
         $this->assertNotEquals($oldGcProbability, $newGcProbability);
         $this->assertEquals(100, $newGcProbability);
+        $session->setGCProbability($oldGcProbability);
     }
 
     /**
@@ -88,4 +91,24 @@ class SessionTest extends TestCase
 
         $session->destroy();
     }
+
+    public function testInitUseStrictMode()
+    {
+        $this->initStrictModeTest(Session::className());
+    }
+
+    public function testUseStrictMode()
+    {
+        //Manual garbage collection since native storage module might not support removing data via Session::destroySession()
+        $sessionSavePath = session_save_path() ?: sys_get_temp_dir();
+        // Only perform garbage collection if "N argument" is not used,
+        // see https://www.php.net/manual/en/session.configuration.php#ini.session.save-path
+        if (strpos($sessionSavePath, ';') === false) {
+            foreach (['non-existing-non-strict', 'non-existing-strict'] as $sessionId) {
+                @unlink($sessionSavePath . '/sess_' . $sessionId);
+            }
+        }
+
+        $this->useStrictModeTest(Session::className());
+    }
 }

tests/framework/web/session/SessionTestTrait.php

@@ -0,0 +1,73 @@
+<?php
+
+
+namespace yiiunit\framework\web\session;
+
+use yii\web\Session;
+
+trait SessionTestTrait
+{
+    public function initStrictModeTest($class)
+    {
+        /** @var Session $session */
+        $session = new $class();
+
+        $session->useStrictMode = false;
+        $this->assertEquals(false, $session->getUseStrictMode());
+
+        if (PHP_VERSION_ID < 50502 && !$session->getUseCustomStorage()) {
+            $this->expectException('yii\base\InvalidConfigException');
+            $session->useStrictMode = true;
+            return;
+        }
+
+        $session->useStrictMode = true;
+        $this->assertEquals(true, $session->getUseStrictMode());
+    }
+
+    /**
+     * @param string $class
+     */
+    protected function useStrictModeTest($class)
+    {
+        /** @var Session $session */
+        $session = new $class();
+
+        if (PHP_VERSION_ID < 50502 && !$session->getUseCustomStorage()) {
+            $this->markTestSkipped('Can not be tested on PHP < 5.5.2 without custom storage class.');
+            return;
+        }
+
+        //non-strict-mode test
+        $session->useStrictMode = false;
+        $session->close();
+        $session->destroySession('non-existing-non-strict');
+        $session->setId('non-existing-non-strict');
+        $session->open();
+        $this->assertEquals('non-existing-non-strict', $session->getId());
+        $session->close();
+
+        //strict-mode test
+        $session->useStrictMode = true;
+        $session->close();
+        $session->destroySession('non-existing-strict');
+        $session->setId('non-existing-strict');
+        $session->open();
+        $id = $session->getId();
+        $this->assertNotEquals('non-existing-strict', $id);
+        $session->set('strict_mode_test', 'session data');
+        $session->close();
+        //Ensure session was not stored under forced id
+        $session->setId('non-existing-strict');
+        $session->open();
+        $this->assertNotEquals('session data', $session->get('strict_mode_test'));
+        $session->close();
+        //Ensure session can be accessed with the new (and thus existing) id.
+        $session->setId($id);
+        $session->open();
+        $this->assertNotEmpty($id);
+        $this->assertEquals($id, $session->getId());
+        $this->assertEquals('session data', $session->get('strict_mode_test'));
+        $session->close();
+    }
+}