Skip to content

Commit

Permalink
[7.x] Fix cross-domain cookie leakage (#3018)
Browse files Browse the repository at this point in the history
Co-authored-by: Tim Düsterhus <209270+TimWolla@users.noreply.github.com>
  • Loading branch information
GrahamCampbell and TimWolla committed May 25, 2022
1 parent b720a2d commit 74a8602
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions README.md
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/Cookie/CookieJar.php
Expand Up @@ -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);
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/Cookie/SetCookie.php
Expand Up @@ -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;
}

Expand Down
41 changes: 35 additions & 6 deletions tests/Cookie/CookieJarTest.php
Expand Up @@ -271,7 +271,7 @@ public function getMatchingCookiesDataProvider()
/**
* @dataProvider getMatchingCookiesDataProvider
*/
public function testReturnsCookiesMatchingRequests($url, $cookies)
public function testReturnsCookiesMatchingRequests(string $url, string $cookies)
{
$bag = [
new SetCookie([
Expand Down Expand Up @@ -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(
Expand All @@ -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);
Expand Down

0 comments on commit 74a8602

Please sign in to comment.