fix: ensure csrf token is string (#9365)

* fix: ensure csrf token is string

* fix: ensure csrf token is string

* handle request php://input

* handle request php://input

* wip

* wip

* wip

* wip

Co-authored-by: Michal Sniatala <michal@sniatala.pl>

* wip

Co-authored-by: Michal Sniatala <michal@sniatala.pl>

* Update user_guide_src/source/changelogs/v4.5.8.rst

* Use data providers

---------

Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Co-authored-by: John Paul E. Balandan, CPA <paulbalandan@gmail.com>
This commit is contained in:
Ngô Quốc Đạt 2025-01-18 21:33:37 +07:00 committed by GitHub
parent 5f8aa24280
commit 97a6d66640
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 94 additions and 22 deletions

View File

@ -307,13 +307,13 @@ class Security implements SecurityInterface
// Does the token exist in POST, HEADER or optionally php:://input - json data or PUT, DELETE, PATCH - raw data.
if ($tokenValue = $request->getPost($this->config->tokenName)) {
return $tokenValue;
return is_string($tokenValue) ? $tokenValue : null;
}
if ($request->hasHeader($this->config->headerName)
&& $request->header($this->config->headerName)->getValue() !== ''
&& $request->header($this->config->headerName)->getValue() !== []) {
return $request->header($this->config->headerName)->getValue();
if ($request->hasHeader($this->config->headerName)) {
$tokenValue = $request->header($this->config->headerName)->getValue();
return (is_string($tokenValue) && $tokenValue !== '') ? $tokenValue : null;
}
$body = (string) $request->getBody();
@ -321,12 +321,15 @@ class Security implements SecurityInterface
if ($body !== '') {
$json = json_decode($body);
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
return $json->{$this->config->tokenName} ?? null;
$tokenValue = $json->{$this->config->tokenName} ?? null;
return is_string($tokenValue) ? $tokenValue : null;
}
parse_str($body, $parsed);
$tokenValue = $parsed[$this->config->tokenName] ?? null;
return $parsed[$this->config->tokenName] ?? null;
return is_string($tokenValue) ? $tokenValue : null;
}
return null;

View File

@ -24,6 +24,7 @@ use CodeIgniter\Test\Mock\MockAppConfig;
use CodeIgniter\Test\Mock\MockSecurity;
use Config\Security as SecurityConfig;
use PHPUnit\Framework\Attributes\BackupGlobals;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;
/**
@ -42,13 +43,23 @@ final class SecurityTest extends CIUnitTestCase
$this->resetServices();
}
private function createMockSecurity(?SecurityConfig $config = null): MockSecurity
private static function createMockSecurity(SecurityConfig $config = new SecurityConfig()): MockSecurity
{
$config ??= new SecurityConfig();
return new MockSecurity($config);
}
private static function createIncomingRequest(): IncomingRequest
{
$config = new MockAppConfig();
return new IncomingRequest(
$config,
new SiteURI($config),
null,
new UserAgent(),
);
}
public function testBasicConfigIsSaved(): void
{
$security = $this->createMockSecurity();
@ -108,18 +119,6 @@ final class SecurityTest extends CIUnitTestCase
$security->verify($request);
}
private function createIncomingRequest(): IncomingRequest
{
$config = new MockAppConfig();
return new IncomingRequest(
$config,
new SiteURI($config),
null,
new UserAgent(),
);
}
public function testCSRFVerifyPostReturnsSelfOnMatch(): void
{
$_SERVER['REQUEST_METHOD'] = 'POST';
@ -315,4 +314,73 @@ final class SecurityTest extends CIUnitTestCase
$this->assertIsString($security->getCookieName());
$this->assertIsBool($security->shouldRedirect());
}
public function testGetPostedTokenReturnsTokenFromPost(): void
{
$_POST['csrf_test_name'] = '8b9218a55906f9dcc1dc263dce7f005a';
$request = $this->createIncomingRequest();
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
}
public function testGetPostedTokenReturnsTokenFromHeader(): void
{
$_POST = [];
$request = $this->createIncomingRequest()->setHeader('X-CSRF-TOKEN', '8b9218a55906f9dcc1dc263dce7f005a');
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
}
public function testGetPostedTokenReturnsTokenFromJsonBody(): void
{
$_POST = [];
$jsonBody = json_encode(['csrf_test_name' => '8b9218a55906f9dcc1dc263dce7f005a']);
$request = $this->createIncomingRequest()->setBody($jsonBody);
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
}
public function testGetPostedTokenReturnsTokenFromFormBody(): void
{
$_POST = [];
$formBody = 'csrf_test_name=8b9218a55906f9dcc1dc263dce7f005a';
$request = $this->createIncomingRequest()->setBody($formBody);
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
}
#[DataProvider('provideGetPostedTokenReturnsNullForInvalidInputs')]
public function testGetPostedTokenReturnsNullForInvalidInputs(string $case, IncomingRequest $request): void
{
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
$this->assertNull(
$method($request),
sprintf('Failed asserting that %s returns null on invalid input.', $case),
);
}
/**
* @return iterable<string, array{string, IncomingRequest}>
*/
public static function provideGetPostedTokenReturnsNullForInvalidInputs(): iterable
{
$testCases = [
'empty_post' => self::createIncomingRequest(),
'invalid_post_data' => self::createIncomingRequest()->setGlobal('post', ['csrf_test_name' => ['invalid' => 'data']]),
'empty_header' => self::createIncomingRequest()->setHeader('X-CSRF-TOKEN', ''),
'invalid_json_data' => self::createIncomingRequest()->setBody(json_encode(['csrf_test_name' => ['invalid' => 'data']])),
'invalid_json' => self::createIncomingRequest()->setBody('{invalid json}'),
'missing_token_in_body' => self::createIncomingRequest()->setBody('other=value&another=test'),
'invalid_form_data' => self::createIncomingRequest()->setBody('csrf_test_name[]=invalid'),
];
foreach ($testCases as $case => $request) {
yield $case => [$case, $request];
}
}
}

View File

@ -39,6 +39,7 @@ Bugs Fixed
**********
- **Database:** Fixed a bug where ``Builder::affectedRows()`` threw an error when the previous query call failed in ``Postgre`` and ``SQLSRV`` drivers.
- **Security:** Fixed a bug where the CSRF token validation could fail on malformed input, causing a generic HTTP 500 status code instead of handling the input gracefully.
See the repo's
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_