Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions system/Security/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions tests/system/Security/SecurityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, array{string}>
*/
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')
Expand Down
4 changes: 4 additions & 0 deletions user_guide_src/source/changelogs/v4.7.6.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.
Loading