diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d19ea86e..241443f2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/RedirectMiddleware.php b/src/RedirectMiddleware.php index 89c06526b..cedad4e77 100644 --- a/src/RedirectMiddleware.php +++ b/src/RedirectMiddleware.php @@ -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. */ @@ -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); @@ -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')) diff --git a/tests/RedirectMiddlewareTest.php b/tests/RedirectMiddlewareTest.php index 1e0841a27..3378d241d 100644 --- a/tests/RedirectMiddlewareTest.php +++ b/tests/RedirectMiddlewareTest.php @@ -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()