From 806ad6c41fdcc9a66c15f2dde0b743e2b6002577 Mon Sep 17 00:00:00 2001 From: Ignace Nyamagana Butera Date: Mon, 12 Sep 2022 06:40:40 +0200 Subject: [PATCH] Normalizes getPath returned value to prevent potential XSS --- CHANGELOG.md | 18 ++++++++++++++++++ src/Http.php | 14 +++++++------- src/HttpTest.php | 36 ++++++++++++++++++++++++++++++++++++ src/Uri.php | 4 ++++ src/UriTest.php | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e457beac..93b0a133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ All Notable changes to `League\Uri` will be documented in this file +## [6.7.2](https://github.com/thephpleague/uri/compare/6.7.1...6.7.2) - 2022-09-12 + +### Added + +- None + +### Fixed + +- `Http::getPath` and `Uri::getPath` methods returned values are normalized to prevent potential XSS and open redirect vectors. + +### Deprecated + +- None + +### Remove + +- None + ## [6.7.1](https://github.com/thephpleague/uri/compare/6.7.0...6.7.1) - 2022-06-29 ### Added diff --git a/src/Http.php b/src/Http.php index daf29c8c..b3b8cea8 100644 --- a/src/Http.php +++ b/src/Http.php @@ -190,7 +190,7 @@ public function withScheme($scheme): self } $uri = $this->uri->withScheme($scheme); - if ($uri->getScheme() === $this->uri->getScheme()) { + if ((string) $uri === (string) $this->uri) { return $this; } @@ -227,7 +227,7 @@ public function withUserInfo($user, $password = null): self } $uri = $this->uri->withUserInfo($user, $password); - if ($uri->getUserInfo() === $this->uri->getUserInfo()) { + if ((string) $uri === (string) $this->uri) { return $this; } @@ -246,7 +246,7 @@ public function withHost($host): self } $uri = $this->uri->withHost($host); - if ($uri->getHost() === $this->uri->getHost()) { + if ((string) $uri === (string) $this->uri) { return $this; } @@ -259,7 +259,7 @@ public function withHost($host): self public function withPort($port): self { $uri = $this->uri->withPort($port); - if ($uri->getPort() === $this->uri->getPort()) { + if ((string) $uri === (string) $this->uri) { return $this; } @@ -272,7 +272,7 @@ public function withPort($port): self public function withPath($path): self { $uri = $this->uri->withPath($path); - if ($uri->getPath() === $this->uri->getPath()) { + if ((string) $uri === (string) $this->uri) { return $this; } @@ -291,7 +291,7 @@ public function withQuery($query): self } $uri = $this->uri->withQuery($query); - if ($uri->getQuery() === $this->uri->getQuery()) { + if ((string) $uri === (string) $this->uri) { return $this; } @@ -310,7 +310,7 @@ public function withFragment($fragment): self } $uri = $this->uri->withFragment($fragment); - if ($uri->getFragment() === $this->uri->getFragment()) { + if ((string) $uri === (string) $this->uri) { return $this; } diff --git a/src/HttpTest.php b/src/HttpTest.php index 10aa961a..a2fee24e 100644 --- a/src/HttpTest.php +++ b/src/HttpTest.php @@ -281,4 +281,40 @@ public function testModificationFailedWithEmptyAuthority(): void ->withHost('') ->withPath('//toto'); } + + public function testItStripMultipleLeadingSlashOnGetPath(): void + { + $uri = Http::createFromString('https://example.com///miscillaneous.tld'); + + self::assertSame('https://example.com///miscillaneous.tld', (string) $uri); + self::assertSame('/miscillaneous.tld', $uri->getPath()); + + $modifiedUri = $uri->withPath('///foobar'); + + self::assertSame('https://example.com///foobar', (string) $modifiedUri); + self::assertSame('/foobar', $modifiedUri->getPath()); + self::assertSame('//example.com///foobar', (string) $modifiedUri->withScheme('')); + + $this->expectException(SyntaxError::class); + + $modifiedUri->withScheme('')->withHost(''); + } + + public function testItPreservesMultipleLeadingSlashesOnMutation(): void + { + $uri = Http::createFromString('https://www.example.com///google.com'); + + self::assertSame('https://www.example.com///google.com', (string) $uri); + self::assertSame('/google.com', $uri->getPath()); + + $modifiedUri = $uri->withPath('/google.com'); + + self::assertSame('https://www.example.com/google.com', (string) $modifiedUri); + self::assertSame('/google.com', $modifiedUri->getPath()); + + $modifiedUri2 = $uri->withPath('///google.com'); + + self::assertSame('https://www.example.com///google.com', (string) $modifiedUri2); + self::assertSame('/google.com', $modifiedUri2->getPath()); + } } diff --git a/src/Uri.php b/src/Uri.php index 42127304..77cb54bb 100644 --- a/src/Uri.php +++ b/src/Uri.php @@ -1143,6 +1143,10 @@ public function getPort(): ?int */ public function getPath(): string { + if (0 === strpos($this->path, '//')) { + return '/'.ltrim($this->path, '/'); + } + return $this->path; } diff --git a/src/UriTest.php b/src/UriTest.php index b11913f7..24195be8 100644 --- a/src/UriTest.php +++ b/src/UriTest.php @@ -601,4 +601,36 @@ public function testIssue171TheEmptySchemeShouldThrow(): void Uri::createFromString('domain.com')->withScheme(''); } + + public function testItStripMultipleLeadingSlashOnGetPath(): void + { + $uri = Uri::createFromString('https://example.com///miscillaneous.tld'); + + self::assertSame('https://example.com///miscillaneous.tld', (string) $uri); + self::assertSame('/miscillaneous.tld', $uri->getPath()); + + $modifiedUri = $uri->withPath('///foobar'); + + self::assertSame('https://example.com///foobar', (string) $modifiedUri); + self::assertSame('/foobar', $modifiedUri->getPath()); + self::assertSame('//example.com///foobar', (string) $modifiedUri->withScheme(null)); + + $this->expectException(SyntaxError::class); + $modifiedUri->withScheme(null)->withHost(null); + } + + public function testItPreservesMultipleLeadingSlashesOnMutation(): void + { + $uri = Uri::createFromString('https://www.example.com///google.com'); + self::assertSame('https://www.example.com///google.com', (string) $uri); + self::assertSame('/google.com', $uri->getPath()); + + $modifiedUri = $uri->withPath('/google.com'); + self::assertSame('https://www.example.com/google.com', (string) $modifiedUri); + self::assertSame('/google.com', $modifiedUri->getPath()); + + $modifiedUri2 = $uri->withPath('///google.com'); + self::assertSame('https://www.example.com///google.com', (string) $modifiedUri2); + self::assertSame('/google.com', $modifiedUri2->getPath()); + } }