From 724562fa861e21a4071c652c8a159934e4f05592 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Thu, 9 Jun 2022 22:36:50 +0100 Subject: [PATCH] Release 6.5.7 (#3022) --- CHANGELOG.md | 5 +++++ src/RedirectMiddleware.php | 38 +++++++++++++++++++++++++++----- tests/RedirectMiddlewareTest.php | 37 +++++++++++++++++-------------- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95d26df21..cd3db22d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Change Log +## 6.5.7 - 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 + ## 6.5.6 - 2022-05-25 * Fix cross-domain cookie leakage diff --git a/src/RedirectMiddleware.php b/src/RedirectMiddleware.php index e4644b7ac..fd86c60a7 100644 --- a/src/RedirectMiddleware.php +++ b/src/RedirectMiddleware.php @@ -141,7 +141,7 @@ function (ResponseInterface $response) use ($uri, $statusCode) { } /** - * Check for too many redirects + * Check for too many redirects. * * @return void * @@ -190,7 +190,7 @@ public function modifyRequest( $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); @@ -210,16 +210,42 @@ public function modifyRequest( $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\modify_request($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. + * + * @return bool + */ + private static function shouldStripSensitiveHeaders( + UriInterface $originalUri, + UriInterface $modifiedUri + ) { + 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. * * @param RequestInterface $request * @param ResponseInterface $response @@ -227,7 +253,7 @@ public function modifyRequest( * * @return UriInterface */ - private function redirectUri( + private static function redirectUri( RequestInterface $request, ResponseInterface $response, array $protocols diff --git a/tests/RedirectMiddlewareTest.php b/tests/RedirectMiddlewareTest.php index e74f713d2..fc830bacb 100644 --- a/tests/RedirectMiddlewareTest.php +++ b/tests/RedirectMiddlewareTest.php @@ -251,31 +251,36 @@ public function testInvokesOnRedirectForRedirects() self::assertTrue($call); } - public function testRemoveAuthorizationHeaderOnRedirect() + public function crossOriginRedirectProvider() { - $mock = new MockHandler([ - new Response(302, ['Location' => 'http://test.com']), - function (RequestInterface $request) { - self::assertFalse($request->hasHeader('Authorization')); - return new Response(200); - } - ]); - $handler = HandlerStack::create($mock); - $client = new Client(['handler' => $handler]); - $client->get('http://example.com?a=b', ['auth' => ['testuser', 'testpass']]); + 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], + ]; } - public function testNotRemoveAuthorizationHeaderOnRedirect() + /** + * @dataProvider crossOriginRedirectProvider + */ + public function testHeadersTreatmentOnRedirect($originalUri, $targetUri, $shouldBePresent) { $mock = new MockHandler([ - new Response(302, ['Location' => 'http://example.com/2']), - function (RequestInterface $request) { - self::assertTrue($request->hasHeader('Authorization')); + new Response(302, ['Location' => $targetUri]), + 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']]); } }