10000 bug #18688 [HttpFoundation] Warning when request has both Forwarded a… · symfony/http-kernel@b4c9467 · GitHub
[go: up one dir, main page]
Skip to content

Commit b4c9467

Browse files
fabpotfabpot
committed
bug #18688 [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For (magnusnordlander)
This PR was squashed before being merged into the 2.7 branch (closes #18688). Discussion ---------- [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#6526 Emit a warning when a request has both a trusted Forwarded header and a trusted X-Forwarded-For header, as this is most likely a misconfiguration which causes security issues. Commits ------- ee8842f [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For
2 parents f5ba3cb + 6634dcc commit b4c9467

File tree

3 files changed

+129
-1
lines changed

3 files changed

+129
-1
lines changed

‎EventListener/ValidateRequestListener.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
16+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
17+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
18+
use Symfony\Component\HttpKernel\KernelEvents;
19+
20+
/**
21+
* Validates that the headers and other information indicating the
22+
* client IP address of a request are consistent.
23+
*
24+
* @author Magnus Nordlander <magnus@fervo.se>
25+
*/
26+
class ValidateRequestListener implements EventSubscriberInterface
27+
{
28+
/**
29+
* Performs the validation.
30+
*
31+
* @param GetResponseEvent $event
32+
*/
33+
public function onKernelRequest(GetResponseEvent $event)
34+
{
35+
if ($event->isMasterRequest()) {
36+
try {
37+
// This will throw an exception if the headers are inconsistent.
38+
$event->getRequest()->getClientIps();
39+
} catch (ConflictingHeadersException $e) {
40+
throw new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e);
41+
}
42+
}
43+
}
44+
45+
/**
46+
* {@inheritdoc}
47+
*/
48+
public static function getSubscribedEvents()
49+
{
50+
return array(
51+
KernelEvents::REQUEST => array(
52+
array('onKernelRequest', 256),
53+
),
54+
);
55+
}
56+
}

‎Profiler/Profiler.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpKernel\Profiler;
1313

14+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1415
use Symfony\Component\HttpFoundation\Request;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface;
@@ -200,9 +201,13 @@ public function collect(Request $request, Response $response, \Exception $except
200201
$profile = new Profile(substr(hash('sha256', uniqid(mt_rand(), true)), 0, 6));
201202
$profile->setTime(time());
202203
$profile->setUrl($request->getUri());
203-
$profile->setIp($request->getClientIp());
204204
$profile->setMethod($request->getMethod());
205205
$profile->setStatusCode($response->getStatusCode());
206+
try {
207+
$profile->setIp($request->getClientIp());
208+
} catch (ConflictingHeadersException $e) {
209+
$profile->setIp('Unknown');
210+
}
206211

207212
$response->headers->set('X-Debug-Token', $profile->getToken());
208213

‎Tests/EventListener/ValidateRequestListenerTest.php

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\Tests\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventDispatcher;
15+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
16+
use Symfony\Component\HttpFoundation\Request;
17+
use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener;
18+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
19+
use Symfony\Component\HttpKernel\HttpKernelInterface;
20+
use Symfony\Component\HttpKernel\KernelEvents;
21+
22+
class ValidateRequestListenerTest extends \PHPUnit_Framework_TestCase
23+
{
24+
public function testListenerThrowsWhenMasterRequestHasInconsistentClientIps()
25+
{
26+
$dispatcher = new EventDispatcher();
27+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
28+
$listener = new ValidateRequestListener();
29+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
30+
$request->method('getClientIps')
31+
->will($this->throwException(new ConflictingHeadersException()));
32+
33+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
34+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
35+
36+
$this->setExpectedException('Symfony\Component\HttpKernel\Exception\BadRequestHttpException');
37+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
38+
}
39+
40+
public function testListenerDoesNothingOnValidRequests()
41+
{
42+
$dispatcher = new EventDispatcher();
43+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
44+
$listener = new ValidateRequestListener();
45+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
46+
$request->method('getClientIps')
47+
->willReturn(array('127.0.0.1'));
48+
49+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
50+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
51+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
52+
}
53+
54+
public function testListenerDoesNothingOnSubrequests()
55+
{
56+
$dispatcher = new EventDispatcher();
57+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
58+
$listener = new ValidateRequestListener();
59+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
60+
$request->method('getClientIps')
61+
->will($this->throwException(new ConflictingHeadersException()));
62+
63+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
64+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST);
65+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
66+
}
67+
}

0 commit comments

Comments
 (0)
0