fix: ensure csrf token is string#9365
fix: ensure csrf token is string#9365paulbalandan merged 11 commits intocodeigniter4:developfrom datlechin:fix-csrf-token
Conversation
|
The same check should be made for |
|
Look at this: private function getPostedToken(RequestInterface $request): ?string
{
assert($request instanceof IncomingRequest);
// 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 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();
}
$body = (string) $request->getBody();
if ($body !== '') {
$json = json_decode($body);
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
return (isset($json->{$this->config->tokenName}) && is_string($json->{$this->config->tokenName}))
? $json->{$this->config->tokenName}
: null;
}
parse_str($body, $parsed);
return (isset($parsed[$this->config->tokenName]) && is_string($parsed[$this->config->tokenName]))
? $parsed[$this->config->tokenName]
: null;
}
return null;
} |
michalsn
left a comment
There was a problem hiding this comment.
Thanks! Please also handle the case where the CSRF token comes from the php://input.
Your commits have to be signed:
- https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits
- https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/signing.md
You can fix the code style with the command:
composer cs-fix
michalsn
left a comment
There was a problem hiding this comment.
Almost there - we also have to check the result of parse_str() function.
And we also need tests for these two scenarios: php://input and parse_str().
|
@datlechin Please run |
michalsn
left a comment
There was a problem hiding this comment.
Please add a chengelog entry here: https://github.com/codeigniter4/CodeIgniter4/blob/develop/user_guide_src/source/changelogs/v4.5.8.rst under "Bugs Fixed".
You can see the previous changelogs as a reference.
|
You need to check here too, the array is unnecessary: if ($request->hasHeader($this->config->headerName)
&& $request->header($this->config->headerName)->getValue() !== ''
&& $request->header($this->config->headerName)->getValue() !== []) {if ($request->hasHeader($this->config->headerName)
&& is_string($request->header($this->config->headerName)->getValue())
&& $request->header($this->config->headerName)->getValue() !== '') {
|
|
@datlechin Please run |
|
Thank you, @datlechin |
Description
Make sure the CSRF token must be a string, because hackers can fake the request and pass csrf_main as an array, the system will break 500.
Checklist: