Skip to content

Commit 8414e8a

Browse files
committed
Merge remote-tracking branch 'upstream/develop' into 4.5
2 parents 8a8865b + 00ba7fb commit 8414e8a

File tree

9 files changed

+954
-20
lines changed

9 files changed

+954
-20
lines changed

phpstan-baseline.php

Lines changed: 849 additions & 9 deletions
Large diffs are not rendered by default.

phpstan.neon.dist

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ parameters:
3535
strictRules:
3636
allRules: false
3737
disallowedLooseComparison: true
38+
booleansInConditions: true
3839
disallowedConstructs: true
3940
matchingInheritedMethodNames: true

system/Router/RouteCollection.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,10 @@ public function loadRoutes(string $routesFile = APPPATH . 'Config/Routes.php')
319319
return $this;
320320
}
321321

322+
// Normalize the path string in routesFile
323+
$realpath = realpath($routesFile);
324+
$routesFile = ($realpath === false) ? $routesFile : $realpath;
325+
322326
// Include the passed in routesFile if it doesn't exist.
323327
// Only keeping that around for BC purposes for now.
324328
$routeFiles = $this->routeFiles;

system/Security/Security.php

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -318,38 +318,53 @@ private function removeTokenInRequest(RequestInterface $request): void
318318
{
319319
assert($request instanceof Request);
320320

321-
$json = json_decode($request->getBody() ?? '');
322-
323321
if (isset($_POST[$this->config->tokenName])) {
324322
// We kill this since we're done and we don't want to pollute the POST array.
325323
unset($_POST[$this->config->tokenName]);
326324
$request->setGlobal('post', $_POST);
327-
} elseif (isset($json->{$this->config->tokenName})) {
328-
// We kill this since we're done and we don't want to pollute the JSON data.
329-
unset($json->{$this->config->tokenName});
330-
$request->setBody(json_encode($json));
325+
} else {
326+
$body = $request->getBody() ?? '';
327+
$json = json_decode($body);
328+
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
329+
// We kill this since we're done and we don't want to pollute the JSON data.
330+
unset($json->{$this->config->tokenName});
331+
$request->setBody(json_encode($json));
332+
} else {
333+
parse_str($body, $parsed);
334+
// We kill this since we're done and we don't want to pollute the BODY data.
335+
unset($parsed[$this->config->tokenName]);
336+
$request->setBody(http_build_query($parsed));
337+
}
331338
}
332339
}
333340

334341
private function getPostedToken(RequestInterface $request): ?string
335342
{
336343
assert($request instanceof IncomingRequest);
337344

338-
// Does the token exist in POST, HEADER or optionally php:://input - json data.
345+
// Does the token exist in POST, HEADER or optionally php:://input - json data or PUT, DELETE, PATCH - raw data.
339346

340347
if ($tokenValue = $request->getPost($this->config->tokenName)) {
341348
return $tokenValue;
342349
}
343350

344-
if ($request->hasHeader($this->config->headerName) && ! empty($request->header($this->config->headerName)->getValue())) {
351+
if ($request->hasHeader($this->config->headerName)
352+
&& $request->header($this->config->headerName)->getValue() !== ''
353+
&& $request->header($this->config->headerName)->getValue() !== []) {
345354
return $request->header($this->config->headerName)->getValue();
346355
}
347356

348357
$body = (string) $request->getBody();
349-
$json = json_decode($body);
350358

351-
if ($body !== '' && ! empty($json) && json_last_error() === JSON_ERROR_NONE) {
352-
return $json->{$this->config->tokenName} ?? null;
359+
if ($body !== '') {
360+
$json = json_decode($body);
361+
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
362+
return $json->{$this->config->tokenName} ?? null;
363+
}
364+
365+
parse_str($body, $parsed);
366+
367+
return $parsed[$this->config->tokenName] ?? null;
353368
}
354369

355370
return null;

tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,19 @@ public function testCSRFVerifyPUTHeaderReturnsSelfOnMatch(): void
246246
$this->assertLogged('info', 'CSRF token verified.');
247247
}
248248

249+
public function testCSRFVerifyPUTBodyReturnsSelfOnMatch(): void
250+
{
251+
$_SERVER['REQUEST_METHOD'] = 'PUT';
252+
253+
$request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent());
254+
$request->setBody("csrf_test_name={$this->randomizedToken}&foo=bar");
255+
256+
$security = $this->createSecurity();
257+
258+
$this->assertInstanceOf(Security::class, $security->verify($request));
259+
$this->assertLogged('info', 'CSRF token verified.');
260+
}
261+
249262
public function testCSRFVerifyJsonThrowsExceptionOnNoMatch(): void
250263
{
251264
$this->expectException(SecurityException::class);

tests/system/Security/SecurityCSRFSessionTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,19 @@ public function testCSRFVerifyPUTHeaderReturnsSelfOnMatch(): void
201201
$this->assertLogged('info', 'CSRF token verified.');
202202
}
203203

204+
public function testCSRFVerifyPUTBodyReturnsSelfOnMatch(): void
205+
{
206+
$_SERVER['REQUEST_METHOD'] = 'PUT';
207+
208+
$request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent());
209+
$request->setBody('csrf_test_name=8b9218a55906f9dcc1dc263dce7f005a&foo=bar');
210+
211+
$security = $this->createSecurity();
212+
213+
$this->assertInstanceOf(Security::class, $security->verify($request));
214+
$this->assertLogged('info', 'CSRF token verified.');
215+
}
216+
204217
public function testCSRFVerifyJsonThrowsExceptionOnNoMatch(): void
205218
{
206219
$this->expectException(SecurityException::class);

tests/system/Security/SecurityTest.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,50 @@ public function testCSRFVerifyJsonReturnsSelfOnMatch(): void
218218
$this->assertSame('{"foo":"bar"}', $request->getBody());
219219
}
220220

221+
public function testCSRFVerifyPutBodyThrowsExceptionOnNoMatch(): void
222+
{
223+
$_SERVER['REQUEST_METHOD'] = 'PUT';
224+
$_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005b';
225+
226+
$security = $this->createMockSecurity();
227+
$request = new IncomingRequest(
228+
new MockAppConfig(),
229+
new URI('http://badurl.com'),
230+
null,
231+
new UserAgent()
232+
);
233+
234+
$request->setBody(
235+
'csrf_test_name=8b9218a55906f9dcc1dc263dce7f005a'
236+
);
237+
238+
$this->expectException(SecurityException::class);
239+
$security->verify($request);
240+
}
241+
242+
public function testCSRFVerifyPutBodyReturnsSelfOnMatch(): void
243+
{
244+
$_SERVER['REQUEST_METHOD'] = 'PUT';
245+
$_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005a';
246+
247+
$security = $this->createMockSecurity();
248+
$request = new IncomingRequest(
249+
new MockAppConfig(),
250+
new URI('http://badurl.com'),
251+
null,
252+
new UserAgent()
253+
);
254+
255+
$request->setBody(
256+
'csrf_test_name=8b9218a55906f9dcc1dc263dce7f005a&foo=bar'
257+
);
258+
259+
$this->assertInstanceOf(Security::class, $security->verify($request));
260+
$this->assertLogged('info', 'CSRF token verified.');
261+
262+
$this->assertSame('foo=bar', $request->getBody());
263+
}
264+
221265
public function testSanitizeFilename(): void
222266
{
223267
$security = $this->createMockSecurity();

user_guide_src/source/changelogs/v4.4.2.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Changes
2222
command was removed. It did not work from the beginning. Also, the rollback
2323
command returns the database(s) state to a specified batch number and cannot
2424
specify only a specific database group.
25+
- **Security:** The presence of the CSRF token is now also checked in the raw body (not JSON format) for PUT, PATCH, and DELETE type of requests.
2526

2627
Deprecations
2728
************

user_guide_src/source/libraries/security.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ The order of checking the availability of the CSRF token is as follows:
204204
1. ``$_POST`` array
205205
2. HTTP header
206206
3. ``php://input`` (JSON request) - bear in mind that this approach is the slowest one since we have to decode JSON and then re-encode it
207+
4. ``php://input`` (raw body) - for PUT, PATCH, and DELETE type of requests
208+
209+
.. note:: ``php://input`` (raw body) is checked since v4.4.2.
207210

208211
*********************
209212
Other Helpful Methods

0 commit comments

Comments
 (0)