-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure csrf token is string #9365
base: develop
Are you sure you want to change the base?
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;
} |
246c279
to
504b62a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
504b62a
to
4087e5b
Compare
@@ -49,6 +51,16 @@ private function createMockSecurity(?SecurityConfig $config = null): MockSecurit | |||
return new MockSecurity($config); | |||
} | |||
|
|||
private function getPostedTokenMethod(): ReflectionMethod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use $this->getPrivateMethod()
instead of making this method.
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: