Skip to content

Commit 08f8131

Browse files
authored
fix: preserve JSON body when CSRF token is sent in header (#10064)
* fix: preserve JSON body when CSRF token is sent in header * add changelog * apply suggestions
1 parent 33efe65 commit 08f8131

File tree

4 files changed

+72
-3
lines changed

4 files changed

+72
-3
lines changed

system/Security/Security.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,15 +298,18 @@ private function removeTokenInRequest(IncomingRequest $request): void
298298
$json = null;
299299
}
300300

301-
if (is_object($json) && property_exists($json, $tokenName)) {
302-
unset($json->{$tokenName});
303-
$request->setBody(json_encode($json));
301+
if (is_object($json)) {
302+
if (property_exists($json, $tokenName)) {
303+
unset($json->{$tokenName});
304+
$request->setBody(json_encode($json));
305+
}
304306

305307
return;
306308
}
307309

308310
// If the token is found in form-encoded data, we can safely remove it.
309311
parse_str($body, $result);
312+
310313
unset($result[$tokenName]);
311314
$request->setBody(http_build_query($result));
312315
}

tests/system/Security/SecurityCSRFSessionTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,37 @@ public function testCSRFVerifyJsonReturnsSelfOnMatch(): void
251251
$this->assertSame('{"foo":"bar"}', $request->getBody());
252252
}
253253

254+
public function testCSRFVerifyHeaderWithJsonBodyPreservesBody(): void
255+
{
256+
service('superglobals')->setServer('REQUEST_METHOD', 'POST');
257+
258+
$request = $this->createIncomingRequest();
259+
$body = '{"foo":"bar"}';
260+
261+
$request->setHeader('X-CSRF-TOKEN', '8b9218a55906f9dcc1dc263dce7f005a');
262+
$request->setBody($body);
263+
$security = $this->createSecurity();
264+
265+
$this->assertInstanceOf(Security::class, $security->verify($request));
266+
$this->assertLogged('info', 'CSRF token verified.');
267+
$this->assertSame($body, $request->getBody());
268+
}
269+
270+
public function testCSRFVerifyHeaderWithJsonBodyStripsTokenFromBody(): void
271+
{
272+
service('superglobals')->setServer('REQUEST_METHOD', 'POST');
273+
274+
$request = $this->createIncomingRequest();
275+
276+
$request->setHeader('X-CSRF-TOKEN', '8b9218a55906f9dcc1dc263dce7f005a');
277+
$request->setBody('{"csrf_test_name":"8b9218a55906f9dcc1dc263dce7f005a","foo":"bar"}');
278+
$security = $this->createSecurity();
279+
280+
$this->assertInstanceOf(Security::class, $security->verify($request));
281+
$this->assertLogged('info', 'CSRF token verified.');
282+
$this->assertSame('{"foo":"bar"}', $request->getBody());
283+
}
284+
254285
public function testRegenerateWithFalseSecurityRegenerateProperty(): void
255286
{
256287
service('superglobals')

tests/system/Security/SecurityTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,39 @@ public function testCsrfVerifyJsonReturnsSelfOnMatch(): void
204204
$this->assertSame('{"foo":"bar"}', $request->getBody());
205205
}
206206

207+
public function testCsrfVerifyHeaderWithJsonBodyPreservesBody(): void
208+
{
209+
service('superglobals')
210+
->setServer('REQUEST_METHOD', 'POST')
211+
->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH);
212+
213+
$security = $this->createMockSecurity();
214+
$request = $this->createIncomingRequest();
215+
$body = '{"foo":"bar"}';
216+
217+
$request->setHeader('X-CSRF-TOKEN', self::CORRECT_CSRF_HASH);
218+
$request->setBody($body);
219+
220+
$this->assertInstanceOf(Security::class, $security->verify($request));
221+
$this->assertSame($body, $request->getBody());
222+
}
223+
224+
public function testCsrfVerifyHeaderWithJsonBodyStripsTokenFromBody(): void
225+
{
226+
service('superglobals')
227+
->setServer('REQUEST_METHOD', 'POST')
228+
->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH);
229+
230+
$security = $this->createMockSecurity();
231+
$request = $this->createIncomingRequest();
232+
233+
$request->setHeader('X-CSRF-TOKEN', self::CORRECT_CSRF_HASH);
234+
$request->setBody('{"csrf_test_name":"' . self::CORRECT_CSRF_HASH . '","foo":"bar"}');
235+
236+
$this->assertInstanceOf(Security::class, $security->verify($request));
237+
$this->assertSame('{"foo":"bar"}', $request->getBody());
238+
}
239+
207240
public function testCsrfVerifyPutBodyThrowsExceptionOnNoMatch(): void
208241
{
209242
service('superglobals')

user_guide_src/source/changelogs/v4.7.2.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ Deprecations
3030
Bugs Fixed
3131
**********
3232

33+
- **Security:** Fixed a bug where the CSRF filter could corrupt JSON request bodies after successful verification when the CSRF token was provided via the ``X-CSRF-TOKEN`` header. This caused ``IncomingRequest::getJSON()`` to fail on valid ``application/json`` requests.
34+
3335
See the repo's
3436
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
3537
for a complete list of bugs fixed.

0 commit comments

Comments
 (0)