diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php
index 78b728ebb83f0d824a131b54bb23c9693a4fd142..11da24a2bc4f52da7006833bff3c6d9f2c436528 100644
--- a/core/lib/Drupal/Core/DrupalKernel.php
+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -1009,6 +1009,7 @@ public static function bootEnvironment($app_root = NULL) {
// Use session cookies, not transparent sessions that puts the session id
// in the query string.
ini_set('session.use_cookies', '1');
+ ini_set('session.use_strict_mode', '1');
if (\PHP_VERSION_ID < 80400) {
ini_set('session.use_only_cookies', '1');
ini_set('session.use_trans_sid', '0');
diff --git a/core/lib/Drupal/Core/Session/SessionHandler.php b/core/lib/Drupal/Core/Session/SessionHandler.php
index fe1247158cd143850eca8c024eafa503907fd8dd..2a17f8f9a51b77e45a900475e4ceb156ce69c8bd 100644
--- a/core/lib/Drupal/Core/Session/SessionHandler.php
+++ b/core/lib/Drupal/Core/Session/SessionHandler.php
@@ -8,12 +8,12 @@
use Drupal\Core\Database\DatabaseException;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy;
+use Symfony\Component\HttpFoundation\Session\Storage\Handler\AbstractSessionHandler;
/**
* Default session handler.
*/
-class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
+class SessionHandler extends AbstractSessionHandler implements \SessionHandlerInterface, \SessionUpdateTimestampHandlerInterface {
use DependencySerializationTrait;
@@ -44,13 +44,13 @@ public function open(string $save_path, string $name): bool {
/**
* {@inheritdoc}
*/
- public function read(#[\SensitiveParameter] string $sid): string|false {
+ public function doRead(#[\SensitiveParameter] string $sessionId): string {
$data = '';
- if (!empty($sid)) {
+ if (!empty($sessionId)) {
try {
// Read the session data from the database.
$query = $this->connection
- ->queryRange('SELECT [session] FROM {sessions} WHERE [sid] = :sid', 0, 1, [':sid' => Crypt::hashBase64($sid)]);
+ ->queryRange('SELECT [session] FROM {sessions} WHERE [sid] = :sid', 0, 1, [':sid' => Crypt::hashBase64($sessionId)]);
$data = (string) $query->fetchField();
}
// Swallow the error if the table hasn't been created yet.
@@ -63,18 +63,18 @@ public function read(#[\SensitiveParameter] string $sid): string|false {
/**
* {@inheritdoc}
*/
- public function write(#[\SensitiveParameter] string $sid, string $value): bool {
+ public function doWrite(#[\SensitiveParameter] string $sessionId, string $data): bool {
$try_again = FALSE;
$request = $this->requestStack->getCurrentRequest();
$fields = [
'uid' => $request->getSession()->get('uid', 0),
'hostname' => $request->getClientIP(),
- 'session' => $value,
+ 'session' => $data,
'timestamp' => $this->time->getRequestTime(),
];
$doWrite = fn() =>
$this->connection->merge('sessions')
- ->keys(['sid' => Crypt::hashBase64($sid)])
+ ->keys(['sid' => Crypt::hashBase64($sessionId)])
->fields($fields)
->execute();
try {
@@ -106,11 +106,18 @@ public function close(): bool {
/**
* {@inheritdoc}
*/
- public function destroy(#[\SensitiveParameter] string $sid): bool {
+ public function destroy(#[\SensitiveParameter] string $sessionId): bool {
+ return $this->doDestroy($sessionId);
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function doDestroy(#[\SensitiveParameter] string $sessionId): bool {
try {
// Delete session data.
$this->connection->delete('sessions')
- ->condition('sid', Crypt::hashBase64($sid))
+ ->condition('sid', Crypt::hashBase64($sessionId))
->execute();
}
// Swallow the error if the table hasn't been created yet.
@@ -140,6 +147,16 @@ public function gc(int $lifetime): int|false {
return FALSE;
}
+ /**
+ * {@inheritdoc}
+ */
+ public function updateTimestamp(#[\SensitiveParameter] string $sessionId, string $data): bool {
+ // This function is intentionally a no-op. Drupal manages session expiry in
+ // the MetadataBag, and the timestamp should not be updated here.
+ // @see \Drupal\Core\Session\MetadataBag::__construct()
+ return TRUE;
+ }
+
/**
* Defines the schema for the session table.
*
diff --git a/core/lib/Drupal/Core/Session/WriteSafeSessionHandler.php b/core/lib/Drupal/Core/Session/WriteSafeSessionHandler.php
index 3c1f61ece10a9814b654c6df985019c326b3548c..b4e39ebd626bd9d68ad0fc026aa1bcabe73c8369 100644
--- a/core/lib/Drupal/Core/Session/WriteSafeSessionHandler.php
+++ b/core/lib/Drupal/Core/Session/WriteSafeSessionHandler.php
@@ -2,15 +2,12 @@
namespace Drupal\Core\Session;
+use Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy;
+
/**
* Wraps the session handler to prevent writes when not necessary or allowed.
*/
-class WriteSafeSessionHandler implements \SessionHandlerInterface, WriteSafeSessionHandlerInterface {
-
- /**
- * @var \SessionHandlerInterface
- */
- protected $wrappedSessionHandler;
+class WriteSafeSessionHandler extends SessionHandlerProxy implements \SessionHandlerInterface, WriteSafeSessionHandlerInterface, \SessionUpdateTimestampHandlerInterface {
/**
* Whether or not the session is enabled for writing.
@@ -30,49 +27,21 @@ class WriteSafeSessionHandler implements \SessionHandlerInterface, WriteSafeSess
/**
* Constructs a new write safe session handler.
*
- * @param \SessionHandlerInterface $wrapped_session_handler
+ * @param \SessionHandlerInterface $handler
* The underlying session handler.
* @param bool $session_writable
* Whether or not the session should be initially writable.
*/
- public function __construct(\SessionHandlerInterface $wrapped_session_handler, $session_writable = TRUE) {
- $this->wrappedSessionHandler = $wrapped_session_handler;
+ public function __construct(\SessionHandlerInterface $handler, $session_writable = TRUE) {
+ parent::__construct($handler);
$this->sessionWritable = $session_writable;
}
/**
* {@inheritdoc}
*/
- public function close(): bool {
- return $this->wrappedSessionHandler->close();
- }
-
- /**
- * {@inheritdoc}
- */
- public function destroy($session_id): bool {
- return $this->wrappedSessionHandler->destroy($session_id);
- }
-
- /**
- * {@inheritdoc}
- */
- public function gc($max_lifetime): int|FALSE {
- return $this->wrappedSessionHandler->gc($max_lifetime);
- }
-
- /**
- * {@inheritdoc}
- */
- public function open($save_path, $session_id): bool {
- return $this->wrappedSessionHandler->open($save_path, $session_id);
- }
-
- /**
- * {@inheritdoc}
- */
- public function read($session_id): string|FALSE {
- $value = $this->wrappedSessionHandler->read($session_id);
+ public function read(#[\SensitiveParameter] string $session_id): string {
+ $value = $this->handler->read($session_id);
$this->readSessions[$session_id] = $value;
return $value;
}
@@ -80,13 +49,13 @@ public function read($session_id): string|FALSE {
/**
* {@inheritdoc}
*/
- public function write($session_id, $session_data): bool {
+ public function write(#[\SensitiveParameter] string $session_id, string $session_data): bool {
// Only write the session when it has been modified.
if (isset($this->readSessions[$session_id]) && $this->readSessions[$session_id] === $session_data) {
return TRUE;
}
if ($this->isSessionWritable()) {
- return $this->wrappedSessionHandler->write($session_id, $session_data);
+ return $this->handler->write($session_id, $session_data);
}
return TRUE;
}
diff --git a/core/modules/system/tests/modules/session_test/session_test.routing.yml b/core/modules/system/tests/modules/session_test/session_test.routing.yml
index fa4b34b0498207363329b0fc34b86d2eee5baf1d..fe85de11032d5da6b7e2a5df2459e12688799d88 100644
--- a/core/modules/system/tests/modules/session_test/session_test.routing.yml
+++ b/core/modules/system/tests/modules/session_test/session_test.routing.yml
@@ -98,6 +98,14 @@ session_test.trace_handler:
requirements:
_access: 'TRUE'
+session_test.trace_handler_rewrite_unmodified:
+ path: '/session-test/trace-handler-rewrite-unmodified'
+ defaults:
+ _title: 'Returns a trace of rewritten but unmodified session data recorded by test proxy session handlers as JSON'
+ _controller: '\Drupal\session_test\Controller\SessionTestController::traceHandlerRewriteUnmodified'
+ requirements:
+ _access: 'TRUE'
+
session_test.get_session_basic_auth:
path: '/session-test/get-session'
defaults:
diff --git a/core/modules/system/tests/modules/session_test/src/Controller/SessionTestController.php b/core/modules/system/tests/modules/session_test/src/Controller/SessionTestController.php
index 9db12e830df371a60bbb2ca62c32f94ce5a9b24b..9c7bb97e24b8575db07af6d2b10027ff70fc8549 100644
--- a/core/modules/system/tests/modules/session_test/src/Controller/SessionTestController.php
+++ b/core/modules/system/tests/modules/session_test/src/Controller/SessionTestController.php
@@ -170,6 +170,43 @@ public function traceHandler(Request $request) {
return new JsonResponse($trace);
}
+ /**
+ * Returns an updated trace recorded by test proxy session handlers as JSON.
+ *
+ * The session data is rewritten without modification to invoke
+ * `\SessionUpdateTimestampHandlerInterface::updateTimestamp`.
+ *
+ * Expects that there is an existing stacked session handler trace as recorded
+ * by `traceHandler()`.
+ *
+ * @param \Symfony\Component\HttpFoundation\Request $request
+ * The incoming request.
+ *
+ * @return \Symfony\Component\HttpFoundation\JsonResponse
+ * The response.
+ *
+ * @throws \AssertionError
+ */
+ public function traceHandlerRewriteUnmodified(Request $request) {
+ // Assert that there is an existing session with stacked handler trace data.
+ assert(
+ is_int($_SESSION['trace-handler']) && $_SESSION['trace-handler'] > 0,
+ 'Existing stacked session handler trace not found'
+ );
+
+ // Save unmodified session data.
+ assert(
+ ini_get('session.lazy_write'),
+ 'session.lazy_write must be enabled to invoke updateTimestamp()'
+ );
+ $request->getSession()->save();
+
+ // Collect traces and return them in JSON format.
+ $trace = \Drupal::service('session_test.session_handler_proxy_trace')->getArrayCopy();
+
+ return new JsonResponse($trace);
+ }
+
/**
* Returns the values stored in the active session and the user ID.
*
diff --git a/core/modules/system/tests/modules/session_test/src/Session/TestSessionHandlerProxy.php b/core/modules/system/tests/modules/session_test/src/Session/TestSessionHandlerProxy.php
index 15d05f4dd0f8b088195fb7a57feee583e89006da..d92031af32fb7667f438dcea7a143b2515cbe995 100644
--- a/core/modules/system/tests/modules/session_test/src/Session/TestSessionHandlerProxy.php
+++ b/core/modules/system/tests/modules/session_test/src/Session/TestSessionHandlerProxy.php
@@ -7,12 +7,12 @@
/**
* Provides a test session handler proxy.
*/
-class TestSessionHandlerProxy implements \SessionHandlerInterface {
+class TestSessionHandlerProxy implements \SessionHandlerInterface, \SessionUpdateTimestampHandlerInterface {
/**
* The decorated session handler.
*
- * @var \SessionHandlerInterface
+ * @var \SessionHandlerInterface&\SessionUpdateTimestampHandlerInterface
*/
protected $sessionHandler;
@@ -31,7 +31,7 @@ class TestSessionHandlerProxy implements \SessionHandlerInterface {
* @param mixed $optional_argument
* (optional) An optional argument.
*/
- public function __construct(\SessionHandlerInterface $session_handler, $optional_argument = NULL) {
+ public function __construct(\SessionHandlerInterface&\SessionUpdateTimestampHandlerInterface $session_handler, $optional_argument = NULL) {
$this->sessionHandler = $session_handler;
$this->optionalArgument = $optional_argument;
}
@@ -94,4 +94,26 @@ public function gc($max_lifetime): int|FALSE {
return $this->sessionHandler->gc($max_lifetime);
}
+ /**
+ * {@inheritdoc}
+ */
+ public function validateId($session_id): bool {
+ $trace = \Drupal::service('session_test.session_handler_proxy_trace');
+ $trace[] = ['BEGIN', $this->optionalArgument, __FUNCTION__, $session_id];
+ $result = $this->sessionHandler->validateId($session_id);
+ $trace[] = ['END', $this->optionalArgument, __FUNCTION__, $session_id];
+ return $result;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function updateTimestamp($session_id, $session_data): bool {
+ $trace = \Drupal::service('session_test.session_handler_proxy_trace');
+ $trace[] = ['BEGIN', $this->optionalArgument, __FUNCTION__, $session_id];
+ $result = $this->sessionHandler->updateTimestamp($session_id, $session_data);
+ $trace[] = ['END', $this->optionalArgument, __FUNCTION__, $session_id];
+ return $result;
+ }
+
}
diff --git a/core/modules/system/tests/src/Functional/Session/SessionTest.php b/core/modules/system/tests/src/Functional/Session/SessionTest.php
index f32462f63a8c11407f6fd1af93e472f9b72beb2b..bf5d3bd409fd1767055dd214f01af845dd0127f1 100644
--- a/core/modules/system/tests/src/Functional/Session/SessionTest.php
+++ b/core/modules/system/tests/src/Functional/Session/SessionTest.php
@@ -4,6 +4,7 @@
namespace Drupal\Tests\system\Functional\Session;
+use Drupal\Component\Utility\Crypt;
use Drupal\Core\Database\Database;
use Drupal\Tests\BrowserTestBase;
@@ -178,6 +179,75 @@ public function testSessionPersistenceOnLogin(): void {
$this->assertSession()->pageTextContains('foobar');
}
+ /**
+ * Tests that an invalid session ID in the cookie is rejected.
+ *
+ * @covers \Drupal\Core\Session\SessionManager::start
+ */
+ public function testAnonymousSessionFixation(): void {
+
+ $mink = $this->getSession();
+ $connection = Database::getConnection();
+
+ // Initialize a session for anonymous user.
+ $this->drupalGet('session-test/set/foo');
+
+ // Switch browser cookie to arbitrary session_id.
+ $session_cookie_name = $this->getSessionName();
+ $initial_session_cookie_value = $mink->getCookie($session_cookie_name);
+
+ $mink->restart();
+ $this->initFrontPage();
+ // Session restart always resets all the cookies by design, so we
+ // set an arbitrary session_id in the cookie for the next request.
+ $invalid_session_cookie_value = bin2hex($this->randomMachineName(13));
+ $mink->setCookie($session_cookie_name, $invalid_session_cookie_value);
+
+ // Make another request.
+ sleep(1);
+ $this->drupalGet('session-test/set/bar');
+
+ // Check returned cookie value.
+ $returned_session_cookie_value = $mink->getCookie($session_cookie_name);
+
+ // The backend should reject $invalid_session_cookie_value and return a
+ // new session_id that's different from both the first and the invalid
+ // SIDs.
+ $this->assertNotEquals(
+ $initial_session_cookie_value,
+ $returned_session_cookie_value,
+ 'Returned session ID is not equal to initial session ID'
+ );
+
+ $this->assertNotEquals(
+ $invalid_session_cookie_value,
+ $returned_session_cookie_value,
+ 'Returned session ID is not equal to invalid session ID'
+ );
+
+ // Check that invalid SID does not exist in database.
+ $this->assertEmpty(
+ $connection
+ ->select('sessions', 's')
+ ->fields('s', ['timestamp'])
+ ->condition('sid', Crypt::hashBase64($invalid_session_cookie_value))
+ ->execute()
+ ->fetchField(),
+ 'Invalid session ID is not in database'
+ );
+
+ // Check that returned SID does exist in database.
+ $this->assertNotEmpty(
+ $connection
+ ->select('sessions', 's')
+ ->fields('s', ['timestamp'])
+ ->condition('sid', Crypt::hashBase64($returned_session_cookie_value))
+ ->execute()
+ ->fetchField(),
+ 'Returned session ID is in database'
+ );
+ }
+
/**
* Tests that empty anonymous sessions are destroyed.
*/
diff --git a/core/modules/system/tests/src/Functional/Session/StackSessionHandlerIntegrationTest.php b/core/modules/system/tests/src/Functional/Session/StackSessionHandlerIntegrationTest.php
index 0f3856140cf949479f949d8dc094d4dbdc62c3d0..4a464d54954809d0bcf4e934df5b721a83508091 100644
--- a/core/modules/system/tests/src/Functional/Session/StackSessionHandlerIntegrationTest.php
+++ b/core/modules/system/tests/src/Functional/Session/StackSessionHandlerIntegrationTest.php
@@ -53,4 +53,96 @@ public function testRequest(): void {
$this->assertEquals($expect_trace, $actual_trace);
}
+ /**
+ * Tests a session modify request with a valid session cookie.
+ *
+ * The trace should include `validateId` because a session cookie is included.
+ *
+ * The trace should include `write` but not include `updateTimestamp` because
+ * the session data is modified.
+ */
+ public function testRequestWriteInvokesValidateId(): void {
+ $options['query'][MainContentViewSubscriber::WRAPPER_FORMAT] = 'drupal_ajax';
+ $headers = ['X-Requested-With' => 'XMLHttpRequest'];
+
+ // Call the write trace handler to store the trace and retrieve a session
+ // cookie.
+ $this->drupalGet('session-test/trace-handler');
+
+ // Call the write trace handler again with the session cookie to modify
+ // the session data.
+ $actual_trace = json_decode($this->drupalGet('session-test/trace-handler', $options, $headers));
+ $sessionId = $this->getSessionCookies()->getCookieByName($this->getSessionName())->getValue();
+
+ $expect_trace = [
+ ["BEGIN", "test_argument", "open"],
+ ["BEGIN", NULL, "open"],
+ ["END", NULL, "open"],
+ ["END", "test_argument", "open"],
+ ["BEGIN", "test_argument", "validateId", $sessionId],
+ ["BEGIN", NULL, "validateId", $sessionId],
+ ["END", NULL, "validateId", $sessionId],
+ ["END", "test_argument", "validateId", $sessionId],
+ ["BEGIN", "test_argument", "read", $sessionId],
+ ["BEGIN", NULL, "read", $sessionId],
+ ["END", NULL, "read", $sessionId],
+ ["END", "test_argument", "read", $sessionId],
+ ["BEGIN", "test_argument", "write", $sessionId],
+ ["BEGIN", NULL, "write", $sessionId],
+ ["END", NULL, "write", $sessionId],
+ ["END", "test_argument", "write", $sessionId],
+ ["BEGIN", "test_argument", "close"],
+ ["BEGIN", NULL, "close"],
+ ["END", NULL, "close"],
+ ["END", "test_argument", "close"],
+ ];
+ $this->assertEquals($expect_trace, $actual_trace);
+ }
+
+ /**
+ * Tests a session rewrite-unmodified request with a valid session cookie.
+ *
+ * The trace should include `validateId` because a session cookie is included.
+ *
+ * The trace should include `updateTimestamp` but not include `write` because
+ * the session data is rewritten without modification and `session.lazy_write`
+ * is enabled.
+ */
+ public function testRequestWriteInvokesUpdateTimestamp(): void {
+ $options['query'][MainContentViewSubscriber::WRAPPER_FORMAT] = 'drupal_ajax';
+ $headers = ['X-Requested-With' => 'XMLHttpRequest'];
+
+ // Call the write trace handler to store the trace and retrieve a session
+ // cookie.
+ $this->drupalGet('session-test/trace-handler');
+
+ // Call the rewrite-unmodified trace handler with the session cookie.
+ $actual_trace = json_decode($this->drupalGet('session-test/trace-handler-rewrite-unmodified', $options, $headers));
+ $sessionId = $this->getSessionCookies()->getCookieByName($this->getSessionName())->getValue();
+
+ $expect_trace = [
+ ["BEGIN", "test_argument", "open"],
+ ["BEGIN", NULL, "open"],
+ ["END", NULL, "open"],
+ ["END", "test_argument", "open"],
+ ["BEGIN", "test_argument", "validateId", $sessionId],
+ ["BEGIN", NULL, "validateId", $sessionId],
+ ["END", NULL, "validateId", $sessionId],
+ ["END", "test_argument", "validateId", $sessionId],
+ ["BEGIN", "test_argument", "read", $sessionId],
+ ["BEGIN", NULL, "read", $sessionId],
+ ["END", NULL, "read", $sessionId],
+ ["END", "test_argument", "read", $sessionId],
+ ["BEGIN", "test_argument", "updateTimestamp", $sessionId],
+ ["BEGIN", NULL, "updateTimestamp", $sessionId],
+ ["END", NULL, "updateTimestamp", $sessionId],
+ ["END", "test_argument", "updateTimestamp", $sessionId],
+ ["BEGIN", "test_argument", "close"],
+ ["BEGIN", NULL, "close"],
+ ["END", NULL, "close"],
+ ["END", "test_argument", "close"],
+ ];
+ $this->assertEquals($expect_trace, $actual_trace);
+ }
+
}