Merge pull request #210 from thephpleague/bugfix/path-security

Normalizes getPath returned value to prevent potential XSS
thephpleague · Sep 13, 2022 · d3b5081 · d3b5081
2 parents 2d7c87a + 806ad6c
commit d3b5081
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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());
+    }
 }