Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 7.4.4 #3023

Merged
merged 2 commits into from Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,11 @@

Please refer to [UPGRADING](UPGRADING.md) guide for upgrading to a major version.

## 7.4.4 - 2022-06-09

* Fix failure to strip Authorization header on HTTP downgrade
* Fix failure to strip the Cookie header on change in host or HTTP downgrade

## 7.4.3 - 2022-05-25

* Fix cross-domain cookie leakage
Expand Down
41 changes: 34 additions & 7 deletions src/RedirectMiddleware.php
Expand Up @@ -142,7 +142,7 @@ static function (ResponseInterface $response) use ($uri, $statusCode) {
}

/**
* Check for too many redirects
* Check for too many redirects.
*
* @throws TooManyRedirectsException Too many redirects.
*/
Expand Down Expand Up @@ -178,7 +178,7 @@ public function modifyRequest(RequestInterface $request, array $options, Respons
$modify['body'] = '';
}

$uri = $this->redirectUri($request, $response, $protocols);
$uri = self::redirectUri($request, $response, $protocols);
if (isset($options['idn_conversion']) && ($options['idn_conversion'] !== false)) {
$idnOptions = ($options['idn_conversion'] === true) ? \IDNA_DEFAULT : $options['idn_conversion'];
$uri = Utils::idnUriConvert($uri, $idnOptions);
Expand All @@ -198,19 +198,46 @@ public function modifyRequest(RequestInterface $request, array $options, Respons
$modify['remove_headers'][] = 'Referer';
}

// Remove Authorization header if host is different.
if ($request->getUri()->getHost() !== $modify['uri']->getHost()) {
// Remove Authorization and Cookie headers if required.
if (self::shouldStripSensitiveHeaders($request->getUri(), $modify['uri'])) {
$modify['remove_headers'][] = 'Authorization';
$modify['remove_headers'][] = 'Cookie';
}

return Psr7\Utils::modifyRequest($request, $modify);
}

/**
* Set the appropriate URL on the request based on the location header
* Determine if we should strip sensitive headers from the request.
*
* We return true if either of the following conditions are true:
*
* 1. the host is different;
* 2. the scheme has changed, and now is non-https.
*/
private function redirectUri(RequestInterface $request, ResponseInterface $response, array $protocols): UriInterface
{
private static function shouldStripSensitiveHeaders(
UriInterface $originalUri,
UriInterface $modifiedUri
): bool {
if (\strcasecmp($originalUri->getHost(), $modifiedUri->getHost()) !== 0) {
return true;
}

if ($originalUri->getScheme() !== $modifiedUri->getScheme() && 'https' !== $modifiedUri->getScheme()) {
return true;
}

return false;
}

/**
* Set the appropriate URL on the request based on the location header.
*/
private static function redirectUri(
RequestInterface $request,
ResponseInterface $response,
array $protocols
): UriInterface {
$location = Psr7\UriResolver::resolve(
$request->getUri(),
new Psr7\Uri($response->getHeaderLine('Location'))
Expand Down
29 changes: 24 additions & 5 deletions tests/RedirectMiddlewareTest.php
Expand Up @@ -272,18 +272,37 @@ public function testInvokesOnRedirectForRedirects()
self::assertTrue($call);
}

public function testRemoveAuthorizationHeaderOnRedirect()
public function crossOriginRedirectProvider()
{
return [
['http://example.com?a=b', 'http://test.com/', false],
['https://example.com?a=b', 'https://test.com/', false],
['http://example.com?a=b', 'https://test.com/', false],
['https://example.com?a=b', 'http://test.com/', false],
['http://example.com?a=b', 'http://example.com/', true],
['https://example.com?a=b', 'https://example.com/', true],
['http://example.com?a=b', 'https://example.com/', true],
['https://example.com?a=b', 'http://example.com/', false],
];
}

/**
* @dataProvider crossOriginRedirectProvider
*/
public function testHeadersTreatmentOnRedirect($originalUri, $targetUri, $shouldBePresent)
{
$mock = new MockHandler([
new Response(302, ['Location' => 'http://test.com']),
static function (RequestInterface $request) {
self::assertFalse($request->hasHeader('Authorization'));
new Response(302, ['Location' => $targetUri]),
static function (RequestInterface $request) use ($shouldBePresent) {
self::assertSame($shouldBePresent, $request->hasHeader('Authorization'));
self::assertSame($shouldBePresent, $request->hasHeader('Cookie'));

return new Response(200);
}
]);
$handler = HandlerStack::create($mock);
$client = new Client(['handler' => $handler]);
$client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass']]);
$client->get($originalUri, ['auth' => ['testuser', 'testpass'], 'headers' => ['Cookie' => 'foo=bar']]);
}

public function testNotRemoveAuthorizationHeaderOnRedirect()
Expand Down