Skip to content

Commit 2b4b0c4

Browse files
committed
bug #59146 [Security] Use the session only if it is started when using SameOriginCsrfTokenManager (Thibault G)
This PR was merged into the 7.2 branch. Discussion ---------- [Security] Use the session only if it is started when using `SameOriginCsrfTokenManager` | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #59092 | License | MIT If I understand well, the `SameOriginCsrfTokenManager` has been created to provide a stateless way of creating CSRF tokens and therefore allow pages with CSRF tokens to be cached. When using `Symfony\Component\Security\Csrf\SameOriginCsrfTokenManager`, I think an additionnal check must be done to ensure that the session is started in addition to verifying that it exists. If not, the CSRF strategy used will be persisted everytime in the session and the stateless check (used with the `#[Route]` attribute parameter) will therefore never pass. Commits ------- 1327e38db3a [Security] Use the session only if it is started when using `SameOriginCsrfTokenManager`
2 parents a2031e5 + e32d3c2 commit 2b4b0c4

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

SameOriginCsrfTokenManager.php

+10-2
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,17 @@ public function clearCookies(Request $request, Response $response): void
207207

208208
public function persistStrategy(Request $request): void
209209
{
210-
if ($request->hasSession(true) && $request->attributes->has($this->cookieName)) {
211-
$request->getSession()->set($this->cookieName, $request->attributes->get($this->cookieName));
210+
if (!$request->attributes->has($this->cookieName)
211+
|| !$request->hasSession(true)
212+
|| !($session = $request->getSession())->isStarted()
213+
) {
214+
return;
212215
}
216+
217+
$usageIndexValue = $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0;
218+
$usageIndexReference = \PHP_INT_MIN;
219+
$session->set($this->cookieName, $request->attributes->get($this->cookieName));
220+
$usageIndexReference = $usageIndexValue;
213221
}
214222

215223
public function onKernelResponse(ResponseEvent $event): void

Tests/SameOriginCsrfTokenManagerTest.php

+16-1
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,11 @@ public function testClearCookies()
221221
$this->assertTrue($response->headers->has('Set-Cookie'));
222222
}
223223

224-
public function testPersistStrategyWithSession()
224+
public function testPersistStrategyWithStartedSession()
225225
{
226226
$session = $this->createMock(Session::class);
227+
$session->method('isStarted')->willReturn(true);
228+
227229
$request = new Request();
228230
$request->setSession($session);
229231
$request->attributes->set('csrf-token', 2 << 8);
@@ -233,6 +235,19 @@ public function testPersistStrategyWithSession()
233235
$this->csrfTokenManager->persistStrategy($request);
234236
}
235237

238+
public function testPersistStrategyWithSessionNotStarted()
239+
{
240+
$session = $this->createMock(Session::class);
241+
242+
$request = new Request();
243+
$request->setSession($session);
244+
$request->attributes->set('csrf-token', 2 << 8);
245+
246+
$session->expects($this->never())->method('set');
247+
248+
$this->csrfTokenManager->persistStrategy($request);
249+
}
250+
236251
public function testOnKernelResponse()
237252
{
238253
$request = new Request([], [], ['csrf-token' => 2], ['csrf-token_test' => 'csrf-token']);

0 commit comments

Comments
 (0)