diff --git a/system/Security/Security.php b/system/Security/Security.php index 4ac0de3f8ff8..583a26593976 100644 --- a/system/Security/Security.php +++ b/system/Security/Security.php @@ -295,23 +295,24 @@ private function removeTokenInRequest(IncomingRequest $request): void try { $json = json_decode($body, flags: JSON_THROW_ON_ERROR); } catch (JsonException) { - $json = null; - } + // The body is not valid JSON - check for form-encoded data. + parse_str($body, $result); - if (is_object($json)) { - if (property_exists($json, $tokenName)) { - unset($json->{$tokenName}); - $request->setBody(json_encode($json)); + if (! array_key_exists($tokenName, $result)) { + return; } + unset($result[$tokenName]); + $request->setBody(http_build_query($result)); + return; } - // If the token is found in form-encoded data, we can safely remove it. - parse_str($body, $result); - - unset($result[$tokenName]); - $request->setBody(http_build_query($result)); + // Valid JSON never falls through to form-encoded processing. + if (is_object($json) && property_exists($json, $tokenName)) { + unset($json->{$tokenName}); + $request->setBody(json_encode($json)); + } } private function getPostedToken(IncomingRequest $request): ?string diff --git a/tests/system/Security/SecurityTest.php b/tests/system/Security/SecurityTest.php index db80d91cbd92..0b8906ee8d34 100644 --- a/tests/system/Security/SecurityTest.php +++ b/tests/system/Security/SecurityTest.php @@ -237,6 +237,41 @@ public function testCsrfVerifyHeaderWithJsonBodyStripsTokenFromBody(): void $this->assertSame('{"foo":"bar"}', $request->getBody()); } + #[DataProvider('provideCsrfVerifyPreservesNonObjectJsonBody')] + public function testCsrfVerifyPreservesNonObjectJsonBody(string $body): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest(); + + $request->setHeader('X-CSRF-TOKEN', self::CORRECT_CSRF_HASH); + $request->setBody($body); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertSame($body, $request->getBody()); + } + + /** + * @return iterable + */ + public static function provideCsrfVerifyPreservesNonObjectJsonBody(): iterable + { + yield 'JSON array' => ['[{"foo":"bar"}]']; + + yield 'JSON array of scalars' => ['[1, 2, 3]']; + + yield 'JSON number' => ['123']; + + yield 'JSON boolean' => ['true']; + + yield 'JSON null' => ['null']; + + yield 'JSON string' => ['"foo"']; + } + public function testCsrfVerifyPutBodyThrowsExceptionOnNoMatch(): void { service('superglobals') diff --git a/user_guide_src/source/changelogs/v4.7.6.rst b/user_guide_src/source/changelogs/v4.7.6.rst index d2641f99b7cf..04eb1be1d52b 100644 --- a/user_guide_src/source/changelogs/v4.7.6.rst +++ b/user_guide_src/source/changelogs/v4.7.6.rst @@ -30,6 +30,10 @@ Deprecations Bugs Fixed ********** +- **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. + See the repo's `CHANGELOG.md `_ for a complete list of bugs fixed.