Skip to content
Open
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
12 changes: 10 additions & 2 deletions system/Security/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,11 @@ private function removeTokenInRequest(IncomingRequest $request): void

// If the token is found in JSON data, we can safely remove it.
try {
$json = json_decode($body, flags: JSON_THROW_ON_ERROR);
$json = json_decode($body, flags: JSON_THROW_ON_ERROR);
$isJson = true;
} catch (JsonException) {
$json = null;
$json = null;
$isJson = false;
}

if (is_object($json)) {
Expand All @@ -307,6 +309,12 @@ private function removeTokenInRequest(IncomingRequest $request): void
return;
}

// A valid JSON body that is not an object cannot contain the CSRF token,
// so leave it untouched.
if ($isJson) {
return;
}

// If the token is found in form-encoded data, we can safely remove it.
parse_str($body, $result);

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