Skip to content

Commit 909cb59

Browse files
committed
bug symfony#59269 [Security/Csrf] Trust "Referer" at the same level as "Origin" (nicolas-grekas)
This PR was merged into the 7.2 branch. Discussion ---------- [Security/Csrf] Trust "Referer" at the same level as "Origin" | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | symfony/demo#1542 | License | MIT As hinted by `@GromNaN` in symfony/demo#1542 (comment), there are proxies that mess up with the `Origin` header, but forward a valid `Referer` header. Since both headers have the same level of trust, I'm proposing to trust them both equally. At the moment, `Origin` overrides `Referer`. With this PR, we check both and accept if just `Referer` matches. Commits ------- 6cd974b [Security/Csrf] Trust "Referer" at the same level as "Origin"
2 parents ebbdb22 + 6cd974b commit 909cb59

File tree

2 files changed

+28
-2
lines changed

2 files changed

+28
-2
lines changed

src/Symfony/Component/Security/Csrf/SameOriginCsrfTokenManager.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,21 @@ public function onKernelResponse(ResponseEvent $event): void
227227
*/
228228
private function isValidOrigin(Request $request): ?bool
229229
{
230-
$source = $request->headers->get('Origin') ?? $request->headers->get('Referer') ?? 'null';
230+
$target = $request->getSchemeAndHttpHost().'/';
231+
$source = 'null';
231232

232-
return 'null' === $source ? null : str_starts_with($source.'/', $request->getSchemeAndHttpHost().'/');
233+
foreach (['Origin', 'Referer'] as $header) {
234+
if (!$request->headers->has($header)) {
235+
continue;
236+
}
237+
$source = $request->headers->get($header);
238+
239+
if (str_starts_with($source.'/', $target)) {
240+
return true;
241+
}
242+
}
243+
244+
return 'null' === $source ? null : false;
233245
}
234246

235247
/**

src/Symfony/Component/Security/Csrf/Tests/SameOriginCsrfTokenManagerTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,20 @@ public function testValidOrigin()
100100
$this->assertSame(1 << 8, $request->attributes->get('csrf-token'));
101101
}
102102

103+
public function testValidRefererInvalidOrigin()
104+
{
105+
$request = new Request();
106+
$request->headers->set('Origin', 'http://localhost:1234');
107+
$request->headers->set('Referer', $request->getSchemeAndHttpHost());
108+
$this->requestStack->push($request);
109+
110+
$token = new CsrfToken('test_token', str_repeat('a', 24));
111+
112+
$this->logger->expects($this->once())->method('debug')->with('CSRF validation accepted using origin info.');
113+
$this->assertTrue($this->csrfTokenManager->isTokenValid($token));
114+
$this->assertSame(1 << 8, $request->attributes->get('csrf-token'));
115+
}
116+
103117
public function testValidOriginAfterDoubleSubmit()
104118
{
105119
$session = $this->createMock(Session::class);

0 commit comments

Comments
 (0)