From 74a8602c6faec9ef74b7a9391ac82c5e65b1cdab Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Wed, 25 May 2022 14:24:33 +0100 Subject: [PATCH] [7.x] Fix cross-domain cookie leakage (#3018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Düsterhus <209270+TimWolla@users.noreply.github.com> --- CHANGELOG.md | 4 ++++ README.md | 14 ++++++------ src/Cookie/CookieJar.php | 5 +++++ src/Cookie/SetCookie.php | 6 +++-- tests/Cookie/CookieJarTest.php | 41 +++++++++++++++++++++++++++++----- 5 files changed, 55 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a078bfbbb..8d19ea86e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Please refer to [UPGRADING](UPGRADING.md) guide for upgrading to a major version. +## 7.4.3 - 2022-05-25 + +* Fix cross-domain cookie leakage + ## 7.4.2 - 2022-03-20 ### Fixed diff --git a/README.md b/README.md index ea0781a66..c96b246eb 100644 --- a/README.md +++ b/README.md @@ -60,13 +60,13 @@ composer require guzzlehttp/guzzle ## Version Guidance -| Version | Status | Packagist | Namespace | Repo | Docs | PSR-7 | PHP Version | -|---------|------------|---------------------|--------------|---------------------|---------------------|-------|-------------| -| 3.x | EOL | `guzzle/guzzle` | `Guzzle` | [v3][guzzle-3-repo] | [v3][guzzle-3-docs] | No | >= 5.3.3 | -| 4.x | EOL | `guzzlehttp/guzzle` | `GuzzleHttp` | [v4][guzzle-4-repo] | N/A | No | >= 5.4 | -| 5.x | EOL | `guzzlehttp/guzzle` | `GuzzleHttp` | [v5][guzzle-5-repo] | [v5][guzzle-5-docs] | No | >= 5.4 | -| 6.x | Security fixes | `guzzlehttp/guzzle` | `GuzzleHttp` | [v6][guzzle-6-repo] | [v6][guzzle-6-docs] | Yes | >= 5.5 | -| 7.x | Latest | `guzzlehttp/guzzle` | `GuzzleHttp` | [v7][guzzle-7-repo] | [v7][guzzle-7-docs] | Yes | >= 7.2 | +| Version | Status | Packagist | Namespace | Repo | Docs | PSR-7 | PHP Version | +|---------|----------------|---------------------|--------------|---------------------|---------------------|-------|--------------| +| 3.x | EOL | `guzzle/guzzle` | `Guzzle` | [v3][guzzle-3-repo] | [v3][guzzle-3-docs] | No | >=5.3.3,<7.0 | +| 4.x | EOL | `guzzlehttp/guzzle` | `GuzzleHttp` | [v4][guzzle-4-repo] | N/A | No | >=5.4,<7.0 | +| 5.x | EOL | `guzzlehttp/guzzle` | `GuzzleHttp` | [v5][guzzle-5-repo] | [v5][guzzle-5-docs] | No | >=5.4,<7.4 | +| 6.x | Security fixes | `guzzlehttp/guzzle` | `GuzzleHttp` | [v6][guzzle-6-repo] | [v6][guzzle-6-docs] | Yes | >=5.5,<8.0 | +| 7.x | Latest | `guzzlehttp/guzzle` | `GuzzleHttp` | [v7][guzzle-7-repo] | [v7][guzzle-7-docs] | Yes | >=7.2.5,<8.2 | [guzzle-3-repo]: https://github.com/guzzle/guzzle3 [guzzle-4-repo]: https://github.com/guzzle/guzzle/tree/4.x diff --git a/src/Cookie/CookieJar.php b/src/Cookie/CookieJar.php index d6757c654..6ef8e8c1d 100644 --- a/src/Cookie/CookieJar.php +++ b/src/Cookie/CookieJar.php @@ -241,6 +241,11 @@ public function extractCookies(RequestInterface $request, ResponseInterface $res if (0 !== \strpos($sc->getPath(), '/')) { $sc->setPath($this->getCookiePathFromRequest($request)); } + if (!$sc->matchesDomain($request->getUri()->getHost())) { + continue; + } + // Note: At this point `$sc->getDomain()` being a public suffix should + // be rejected, but we don't want to pull in the full PSL dependency. $this->setCookie($sc); } } diff --git a/src/Cookie/SetCookie.php b/src/Cookie/SetCookie.php index 7c04034d2..a613c77bf 100644 --- a/src/Cookie/SetCookie.php +++ b/src/Cookie/SetCookie.php @@ -379,10 +379,12 @@ public function matchesDomain(string $domain): bool // Remove the leading '.' as per spec in RFC 6265. // https://tools.ietf.org/html/rfc6265#section-5.2.3 - $cookieDomain = \ltrim($cookieDomain, '.'); + $cookieDomain = \ltrim(\strtolower($cookieDomain), '.'); + + $domain = \strtolower($domain); // Domain not set or exact match. - if (!$cookieDomain || !\strcasecmp($domain, $cookieDomain)) { + if ('' === $cookieDomain || $domain === $cookieDomain) { return true; } diff --git a/tests/Cookie/CookieJarTest.php b/tests/Cookie/CookieJarTest.php index 66f9b4a26..0b67c665c 100644 --- a/tests/Cookie/CookieJarTest.php +++ b/tests/Cookie/CookieJarTest.php @@ -271,7 +271,7 @@ public function getMatchingCookiesDataProvider() /** * @dataProvider getMatchingCookiesDataProvider */ - public function testReturnsCookiesMatchingRequests($url, $cookies) + public function testReturnsCookiesMatchingRequests(string $url, string $cookies) { $bag = [ new SetCookie([ @@ -386,16 +386,13 @@ public function getCookiePathsDataProvider() ['/foo', '/'], ['/foo/bar', '/foo'], ['/foo/bar/', '/foo/bar'], - ['foo', '/'], - ['foo/bar', '/'], - ['foo/bar/', '/'], ]; } /** * @dataProvider getCookiePathsDataProvider */ - public function testCookiePathWithEmptySetCookiePath($uriPath, $cookiePath) + public function testCookiePathWithEmptySetCookiePath(string $uriPath, string $cookiePath) { $response = (new Response(200)) ->withAddedHeader( @@ -407,13 +404,45 @@ public function testCookiePathWithEmptySetCookiePath($uriPath, $cookiePath) "bar=foo; expires={$this->futureExpirationDate()}; domain=www.example.com; path=foobar;" ) ; - $request = (new Request('GET', $uriPath))->withHeader('Host', 'www.example.com'); + $request = (new Request('GET', "https://www.example.com{$uriPath}")); $this->jar->extractCookies($request, $response); self::assertSame($cookiePath, $this->jar->toArray()[0]['Path']); self::assertSame($cookiePath, $this->jar->toArray()[1]['Path']); } + public function getDomainMatchesProvider() + { + return [ + ['www.example.com', 'www.example.com', true], + ['www.example.com', 'www.EXAMPLE.com', true], + ['www.example.com', 'www.example.net', false], + ['www.example.com', 'ftp.example.com', false], + ['www.example.com', 'example.com', true], + ['www.example.com', 'EXAMPLE.com', true], + ['fra.de.example.com', 'EXAMPLE.com', true], + ['www.EXAMPLE.com', 'www.example.com', true], + ['www.EXAMPLE.com', 'www.example.COM', true], + ]; + } + + /** + * @dataProvider getDomainMatchesProvider + */ + public function testIgnoresCookiesForMismatchingDomains(string $requestHost, string $domainAttribute, bool $matches) + { + $response = (new Response(200)) + ->withAddedHeader( + 'Set-Cookie', + "foo=bar; expires={$this->futureExpirationDate()}; domain={$domainAttribute}; path=/;" + ) + ; + $request = (new Request('GET', "https://{$requestHost}/")); + $this->jar->extractCookies($request, $response); + + self::assertCount($matches ? 1 : 0, $this->jar->toArray()); + } + private function futureExpirationDate() { return (new DateTimeImmutable)->add(new DateInterval('P1D'))->format(DateTime::COOKIE);