Skip to content

Commit

Permalink
security #cve-2020-15094 Remove headers with internal meaning from Ht…
Browse files Browse the repository at this point in the history
…tpClient responses (mpdude)

This PR was merged into the 4.4 branch.
  • Loading branch information
fabpot committed Sep 2, 2020
2 parents bca14aa + ba39753 commit d9910e0
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
68 changes: 68 additions & 0 deletions src/Symfony/Component/HttpClient/Tests/CachingHttpClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\HttpClient\Response\MockResponse;
use Symfony\Component\HttpKernel\HttpCache\Store;
use Symfony\Contracts\HttpClient\ResponseInterface;

class CachingHttpClientTest extends TestCase
{
Expand All @@ -39,4 +40,71 @@ public function testRequestHeaders()
self::assertSame($response->getRequestOptions()['normalized_headers']['application-name'][0], 'Application-Name: test1234');
self::assertSame($response->getRequestOptions()['normalized_headers']['test-name-header'][0], 'Test-Name-Header: test12345');
}

public function testDoesNotEvaluateResponseBody()
{
$body = file_get_contents(__DIR__.'/Fixtures/assertion_failure.php');
$response = $this->runRequest(new MockResponse($body, ['response_headers' => ['X-Body-Eval' => true]]));
$headers = $response->getHeaders();

$this->assertSame($body, $response->getContent());
$this->assertArrayNotHasKey('x-body-eval', $headers);
}

public function testDoesNotIncludeFile()
{
$file = __DIR__.'/Fixtures/assertion_failure.php';

$response = $this->runRequest(new MockResponse(
'test', ['response_headers' => [
'X-Body-Eval' => true,
'X-Body-File' => $file,
]]
));
$headers = $response->getHeaders();

$this->assertSame('test', $response->getContent());
$this->assertArrayNotHasKey('x-body-eval', $headers);
$this->assertArrayNotHasKey('x-body-file', $headers);
}

public function testDoesNotReadFile()
{
$file = __DIR__.'/Fixtures/assertion_failure.php';

$response = $this->runRequest(new MockResponse(
'test', ['response_headers' => [
'X-Body-File' => $file,
]]
));
$headers = $response->getHeaders();

$this->assertSame('test', $response->getContent());
$this->assertArrayNotHasKey('x-body-file', $headers);
}

public function testRemovesXContentDigest()
{
$response = $this->runRequest(new MockResponse(
'test', [
'response_headers' => [
'X-Content-Digest' => 'some-hash',
]
]));
$headers = $response->getHeaders();

$this->assertArrayNotHasKey('x-content-digest', $headers);
}

private function runRequest(MockResponse $mockResponse): ResponseInterface
{
$mockClient = new MockHttpClient($mockResponse);

$store = new Store(sys_get_temp_dir() . '/sf_http_cache');
$client = new CachingHttpClient($mockClient, $store);

$response = $client->request('GET', 'http://test');

return $response;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

throw new \PHPUnit\Framework\AssertionFailedError('Response body should not be evaluated.');
4 changes: 4 additions & 0 deletions src/Symfony/Component/HttpKernel/HttpClientKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ

$response = new Response($response->getContent(!$catch), $response->getStatusCode(), $response->getHeaders(!$catch));

$response->headers->remove('X-Body-File');
$response->headers->remove('X-Body-Eval');
$response->headers->remove('X-Content-Digest');

$response->headers = new class($response->headers->all()) extends ResponseHeaderBag {
protected function computeCacheControlValue(): string
{
Expand Down

5 comments on commit d9910e0

@lade-devs
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symfony fix

@leonardobussi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets

@paolotormon
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not working

image

@apfelbox
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paolotormon you can see the versions (tags) where this fix is included here:

2020-10-13 at 10 42

You are still using 5.0.11 (which is from the 5.0 branch, that is not maintained anymore), so you need to update to at least 5.1.5 to fix this particular issue.

(But you should directly update to the newest 5.1 version 😉 )

@paolotormon
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apfelbox thanks! That's so weird because I downloaded directly from the website during that same day.

Anyway, thanks!

Please sign in to comment.