From 345243814eb8edc02123c46ff62541955d5915ae Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 21 Oct 2026 21:43:44 +0200 Subject: [PATCH 1/4] fix(Router): fully decode URI before checking disallowed characters A double-encoded URI could pass checkDisallowedChars() because the Router decoded it only once. Any component decoding it again could then introduce disallowed characters. Decode the URI until it stops changing before validation. The single-decoded path is still used for routing so that URI filter matching stays consistent. --- system/Router/Router.php | 8 +++++++ tests/system/Router/RouterTest.php | 23 +++++++++++++++++++++ user_guide_src/source/changelogs/v4.7.6.rst | 3 +++ user_guide_src/source/general/urls.rst | 5 +++-- 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/system/Router/Router.php b/system/Router/Router.php index f0f9555ad1ac..5b6e877e7834 100644 --- a/system/Router/Router.php +++ b/system/Router/Router.php @@ -759,6 +759,14 @@ protected function setMatchedRoute(string $route, $handler): void */ private function checkDisallowedChars(string $uri): void { + // Decode the URI until it no longer changes. A URI can be encoded + // multiple times, and decoding it only once would allow a + // double-encoded URI to bypass this check when it is decoded again + // by another component. + while (($decoded = urldecode($uri)) !== $uri) { + $uri = $decoded; + } + foreach (explode('/', $uri) as $segment) { if ($segment !== '' && $this->permittedURIChars !== '' && preg_match('/\A[' . $this->permittedURIChars . ']+\z/iu', $segment) !== 1 diff --git a/tests/system/Router/RouterTest.php b/tests/system/Router/RouterTest.php index 6e22c95ed4f9..af1d99944bf9 100644 --- a/tests/system/Router/RouterTest.php +++ b/tests/system/Router/RouterTest.php @@ -120,6 +120,29 @@ public function testNotPermittedChars(): void $router->handle('test/%3Ca%3E'); } + public function testDoubleEncodedNotPermittedChars(): void + { + $router = new Router($this->collection, $this->request); + + $this->expectException(BadRequestException::class); + $this->expectExceptionMessage('The URI you submitted has disallowed characters: ""'); + + $router->handle('test/%253Ca%253E'); + } + + public function testPermittedCharsWithPercentSign(): void + { + $this->collection->get('encoded/(:any)', 'News::view/$1'); + + $router = new Router($this->collection, $this->request); + + $router->handle('encoded/100%25'); + + $this->assertSame('\News', $router->controllerName()); + $this->assertSame('view', $router->methodName()); + $this->assertSame(['100%'], $router->params()); + } + public function testURIMapsToController(): void { $router = new Router($this->collection, $this->request); diff --git a/user_guide_src/source/changelogs/v4.7.6.rst b/user_guide_src/source/changelogs/v4.7.6.rst index 8739e9546d4c..9b7142387c68 100644 --- a/user_guide_src/source/changelogs/v4.7.6.rst +++ b/user_guide_src/source/changelogs/v4.7.6.rst @@ -38,6 +38,9 @@ Bugs Fixed is used instead. Values that cannot be converted to a string, such as objects not implementing ``Stringable``, now throw a ``FormatException`` instead of a fatal ``Error``. +- **Router:** Fixed a bug where a double-encoded URI could bypass the + disallowed character check when the URI was decoded again by another + component. - **Security:** Fixed a bug where a valid JSON request body that is not an object (for example a JSON array) was corrupted after CSRF verification when the token was sent via the ``X-CSRF-TOKEN`` header. diff --git a/user_guide_src/source/general/urls.rst b/user_guide_src/source/general/urls.rst index bdb10508393f..5681a5f039aa 100644 --- a/user_guide_src/source/general/urls.rst +++ b/user_guide_src/source/general/urls.rst @@ -86,8 +86,9 @@ data can be passed to your application. URIs may only contain the following: .. note:: This check is performed by the ``Router``. The Router takes the URL-encoded - value held by the ``SiteURI`` class, decodes it, and then checks that it - does not contain not permitted strings. + value held by the ``SiteURI`` class, decodes it repeatedly until it no + longer changes, and then checks that it does not contain not permitted + strings. Adding Permitted Characters --------------------------- From daa7b7596463aa5e2288382ac32f334c17d8bd99 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 21 Oct 2026 23:11:54 +0200 Subject: [PATCH 2/4] Trigger Tests From 21439823a7a98992093a82941b9fb328db4de1b5 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 21 Oct 2026 23:57:55 +0200 Subject: [PATCH 3/4] Trigger Tests From df756fcb8a13bb8b9e859e5ff927c88ce3976547 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 22 Oct 2026 00:12:47 +0200 Subject: [PATCH 4/4] Trigger Tests