From a63c6015e6900dc3eb0d3d6fb4baa8a683e4eecc Mon Sep 17 00:00:00 2001 From: Nafis Ahmed Date: Wed, 22 Oct 2025 13:04:04 +0600 Subject: [PATCH 1/5] fix: improve previousPath behavior for security and origin validation --- src/Illuminate/Routing/UrlGenerator.php | 67 +++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index 4808c1c0a89e..92e097672baf 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -175,16 +175,77 @@ public function previous($fallback = false) } /** - * Get the previous path info for the request. + * Get the previous path for the request. * * @param mixed $fallback * @return string */ public function previousPath($fallback = false) { - $previousPath = str_replace($this->to('/'), '', rtrim(preg_replace('/\?.*/', '', $this->previous($fallback)), '/')); + $referrer = $this->request->headers->get('referer'); + + if (! $referrer) { + $referrer = $this->getPreviousUrlFromSession(); + } + + if (! $referrer || $this->isDangerousUrl($referrer)) { + $path = $fallback ? parse_url($this->to($fallback), PHP_URL_PATH) ?? '/' : '/'; + return rtrim($path, '/') ?: '/'; + } + + $previous = $this->to($referrer); + + $previousUrlComponents = parse_url($previous); + $appUrlComponents = parse_url($this->to('/')); + + if (! $this->isSameOrigin($previousUrlComponents, $appUrlComponents)) { + $previous = $fallback ? $this->to($fallback) : $this->to('/'); + } + + $path = parse_url($previous, PHP_URL_PATH) ?? '/'; + + return rtrim($path, '/') ?: '/'; + } + + /** + * Check for dangerous schemes like javascript, data, file, or vbscript. + * + * @param string $url + * @return bool + */ + protected function isDangerousUrl($url) + { + // will return true if the URL starts with javascript, data, file, or vbscript + return preg_match('/^(javascript|data|file|vbscript):/i', $url); + } + + /** + * Determine if two URLs are from the same origin, matching host, scheme, and port. + * + * @param array|false $urlOneComponents + * @param array|false $urlTwoComponents + * @return bool + */ + protected function isSameOrigin($urlOneComponents, $urlTwoComponents) + { + if ($urlOneComponents === false || $urlTwoComponents === false) { + return false; + } + + $hostOne = $urlOneComponents['host'] ?? null; + $hostTwo = $urlTwoComponents['host'] ?? null; + $schemeOne = $urlOneComponents['scheme'] ?? null; + $schemeTwo = $urlTwoComponents['scheme'] ?? null; + $portOne = $urlOneComponents['port'] ?? null; + $portTwo = $urlTwoComponents['port'] ?? null; + + if (! $hostOne || ! $hostTwo || ! $schemeOne || ! $schemeTwo) { + return false; + } - return $previousPath === '' ? '/' : $previousPath; + return strtolower($hostOne) === strtolower($hostTwo) && + $schemeOne === $schemeTwo && + $portOne === $portTwo; } /** From 472a810c5a4d809cc94ee2f3f80481416658e85f Mon Sep 17 00:00:00 2001 From: Nafis Ahmed Date: Wed, 22 Oct 2025 13:07:20 +0600 Subject: [PATCH 2/5] test: add thorough tests for UrlGenerator previousPath behavior, security and edge cases --- .../RoutingUrlGeneratorPreviousPathTest.php | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 tests/Routing/RoutingUrlGeneratorPreviousPathTest.php diff --git a/tests/Routing/RoutingUrlGeneratorPreviousPathTest.php b/tests/Routing/RoutingUrlGeneratorPreviousPathTest.php new file mode 100644 index 000000000000..ec08edf8c7e0 --- /dev/null +++ b/tests/Routing/RoutingUrlGeneratorPreviousPathTest.php @@ -0,0 +1,249 @@ +headers->set('referer', $referer); + } + + return new UrlGenerator($routes, $request); + } + + public function testPreviousPathWithSameDomainUrl() + { + $url = $this->getUrlGenerator('http://www.foo.com/bar/baz?query=value'); + + $this->assertSame('/bar/baz', $url->previousPath()); + } + + public function testPreviousPathWithSameDomainRootUrl() + { + $url = $this->getUrlGenerator('http://www.foo.com/'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathWithSameDomainUrlAndQueryString() + { + $url = $this->getUrlGenerator('http://www.foo.com/products/123?category=electronics&sort=price'); + + $this->assertSame('/products/123', $url->previousPath()); + } + + public function testPreviousPathWithSameDomainUrlAndFragment() + { + $url = $this->getUrlGenerator('http://www.foo.com/docs/api#authentication'); + + $this->assertSame('/docs/api', $url->previousPath()); + } + + public function testPreviousPathBlocksExternalDomain() + { + $url = $this->getUrlGenerator('https://evil-site.com/malicious/path'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathBlocksExternalDomainWithAnyPaths() + { + $url = $this->getUrlGenerator('https://attacker.com/admin/users'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathBlocksDifferentScheme() + { + $url = $this->getUrlGenerator('https://www.foo.com/secure/area', 'www.foo.com', 'http'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathAllowsSameSchemeWithPaths() + { + $url = $this->getUrlGenerator('https://www.foo.com/secures/area', 'www.foo.com', 'https'); + + $this->assertSame('/secures/area', $url->previousPath()); + } + + public function testPreviousPathBlocksSubdomainAttack() + { + $url = $this->getUrlGenerator('http://sub.foo.com/malicious'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathBlocksDifferentPortBasedAttack() + { + $url = $this->getUrlGenerator('http://www.foo.com:8080/admin', 'www.foo.com', 'http'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathAllowsSameDomainWithSamePortUrl() + { + $url = $this->getUrlGenerator('http://www.foo.com:8080/allowed', 'www.foo.com:8080', 'http'); + + $this->assertSame('/allowed', $url->previousPath()); + } + + public function testPreviousPathHandlesEmptyReferer() + { + $url = $this->getUrlGenerator(''); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathHandlesNullReferer() + { + $url = $this->getUrlGenerator(null); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathWithFallbackForExternalUrl() + { + $url = $this->getUrlGenerator('https://evil.com/malicious'); + + $this->assertSame('/dashboard', $url->previousPath('/dashboard')); + } + + public function testPreviousPathNormalizesTrailingSlashes() + { + $url = $this->getUrlGenerator('http://www.foo.com/path/to/resource/'); + + $this->assertSame('/path/to/resource', $url->previousPath()); + } + + public function testPreviousPathHandlesDeepNestedPaths() + { + $url = $this->getUrlGenerator('http://www.foo.com/level1/level2/level3/level4/list'); + + $this->assertSame('/level1/level2/level3/level4/list', $url->previousPath()); + } + + public function testPreviousPathHandlesSpecialCharactersInPath() + { + $url = $this->getUrlGenerator('http://www.foo.com/path-with-dashes/under_scores/123'); + + $this->assertSame('/path-with-dashes/under_scores/123', $url->previousPath()); + } + + public function testPreviousPathBlocksDataUri() + { + $url = $this->getUrlGenerator('data:text/html,'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathBlocksJavascriptUri() + { + $url = $this->getUrlGenerator('javascript:alert("xss")'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathAllowsJavascriptUriIfOriginIsSame() + { + $url = $this->getUrlGenerator('http://www.foo.com/javascript:alert("same-origin")', 'www.foo.com'); + + $this->assertSame('/javascript:alert("same-origin")', $url->previousPath()); + } + + public function testPreviousPathBlocksFileUri() + { + $url = $this->getUrlGenerator('file:///etc/passwd'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathAllowsFileUriIfOriginIsSame() + { + $url = $this->getUrlGenerator('http://www.foo.com/file:///etc/same'); + + $this->assertSame('/file:///etc/same', $url->previousPath()); + } + + public function testPreviousPathHandlesCaseInsensitiveHost() + { + $url = $this->getUrlGenerator('http://WWW.FOO.COM/path', 'www.foo.com'); + + $this->assertSame('/path', $url->previousPath()); + } + + public function testPreviousPathHandlesCaseVariations() + { + $url = $this->getUrlGenerator('http://www.FOO.com/path', 'www.foo.com'); + + $this->assertSame('/path', $url->previousPath()); + } + + public function testPreviousPathHandlesUrlWithoutPath() + { + $url = $this->getUrlGenerator('http://www.foo.com'); + + $this->assertSame('/', $url->previousPath()); + } + + public function testPreviousPathHandlesComplexQueryString() + { + $url = $this->getUrlGenerator('http://www.foo.com/search?q=php&framework=laravel&sort=popularity&page=34'); + + $this->assertSame('/search', $url->previousPath()); + } + + public function testPreviousPathHandlesEncodedCharacters() + { + $url = $this->getUrlGenerator('http://www.foo.com/path%20with%20spaces%20encoded/resource'); + + $this->assertSame('/path%20with%20spaces%20encoded/resource', $url->previousPath()); + } + + public function testIsSameOriginMethod() + { + $url = $this->getUrlGenerator(); + + $reflection = new \ReflectionClass($url); + $method = $reflection->getMethod('isSameOrigin'); + $method->setAccessible(true); + + $this->assertTrue($method->invoke($url, + ['host' => 'www.foo.com', 'scheme' => 'http'], + ['host' => 'www.foo.com', 'scheme' => 'http'] + )); + + // different host + $this->assertFalse($method->invoke($url, + ['host' => 'evil.com', 'scheme' => 'http'], + ['host' => 'www.foo.com', 'scheme' => 'http'] + )); + + // different scheme + $this->assertFalse($method->invoke($url, + ['host' => 'www.foo.com', 'scheme' => 'https'], + ['host' => 'www.foo.com', 'scheme' => 'http'] + )); + + // missing component + $this->assertFalse($method->invoke($url, false, ['host' => 'www.foo.com', 'scheme' => 'http'])); + $this->assertFalse($method->invoke($url, ['host' => 'www.foo.com'], ['host' => 'www.foo.com', 'scheme' => 'http'])); + + // case insensitive + $this->assertTrue($method->invoke($url, + ['host' => 'WWW.FOO.COM', 'scheme' => 'http'], + ['host' => 'www.foo.com', 'scheme' => 'http'] + )); + } +} From 0bcf65cec948b560493dcba1fb6deecb55cbbc8f Mon Sep 17 00:00:00 2001 From: Nafis Ahmed Date: Wed, 22 Oct 2025 13:17:07 +0600 Subject: [PATCH 3/5] add some comments for clarity --- src/Illuminate/Routing/UrlGenerator.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index 92e097672baf..cf4ce00fcae7 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -188,6 +188,8 @@ public function previousPath($fallback = false) $referrer = $this->getPreviousUrlFromSession(); } + // checking the referrer against dangerous schemes (e.g., javascript:, data:, file: etc) + // to prevent open redirect vulnerabilities if (! $referrer || $this->isDangerousUrl($referrer)) { $path = $fallback ? parse_url($this->to($fallback), PHP_URL_PATH) ?? '/' : '/'; return rtrim($path, '/') ?: '/'; @@ -198,6 +200,7 @@ public function previousPath($fallback = false) $previousUrlComponents = parse_url($previous); $appUrlComponents = parse_url($this->to('/')); + // if the previous URL is not from the same origin, we will use the fallback or root URL if (! $this->isSameOrigin($previousUrlComponents, $appUrlComponents)) { $previous = $fallback ? $this->to($fallback) : $this->to('/'); } From 59a511b6f73cd809d611b2205e6750eee9d4ea3c Mon Sep 17 00:00:00 2001 From: Nafis Ahmed Date: Wed, 22 Oct 2025 14:15:13 +0600 Subject: [PATCH 4/5] fix styleCI issue --- src/Illuminate/Routing/UrlGenerator.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index cf4ce00fcae7..69fc386bbc86 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -192,6 +192,7 @@ public function previousPath($fallback = false) // to prevent open redirect vulnerabilities if (! $referrer || $this->isDangerousUrl($referrer)) { $path = $fallback ? parse_url($this->to($fallback), PHP_URL_PATH) ?? '/' : '/'; + return rtrim($path, '/') ?: '/'; } From d2e999cc0c66712e687151d1646cd801c331671e Mon Sep 17 00:00:00 2001 From: Nafis Ahmed Date: Wed, 22 Oct 2025 14:16:49 +0600 Subject: [PATCH 5/5] fix styleCI issue again --- src/Illuminate/Routing/UrlGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index 69fc386bbc86..19879d569a40 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -192,7 +192,7 @@ public function previousPath($fallback = false) // to prevent open redirect vulnerabilities if (! $referrer || $this->isDangerousUrl($referrer)) { $path = $fallback ? parse_url($this->to($fallback), PHP_URL_PATH) ?? '/' : '/'; - + return rtrim($path, '/') ?: '/'; }