Skip to content

Commit 2929934

Browse files
authored
Merge commit from fork
* fix: validate client extension in ext_in upload rule * add changelog and upgrade notes
1 parent b186ed4 commit 2929934

5 files changed

Lines changed: 119 additions & 3 deletions

File tree

system/Validation/StrictRules/FileRules.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,14 @@ public function ext_in(?string $blank, string $params): bool
211211
return true;
212212
}
213213

214-
if (! in_array($file->guessExtension(), $params, true)) {
214+
// Check the real filename extension, not only the guessed extension.
215+
$clientExtension = strtolower($file->getClientExtension());
216+
217+
if ($clientExtension === '' || ! in_array($clientExtension, $params, true)) {
218+
return false;
219+
}
220+
221+
if ($file->guessExtension() !== $clientExtension) {
215222
return false;
216223
}
217224
}

tests/system/Validation/StrictRules/FileRulesTest.php

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,20 @@ public function testExtensionOk(): void
316316
$this->assertTrue($this->validation->run([]));
317317
}
318318

319+
public function testExtensionOkWithMatchingClientExtensionAndMimeType(): void
320+
{
321+
$payload = $this->createGifPayload();
322+
323+
try {
324+
$this->setUploadedAvatar($payload, 'my-avatar.gif');
325+
326+
$this->validation->setRules(['avatar' => 'ext_in[avatar,gif]']);
327+
$this->assertTrue($this->validation->run([]));
328+
} finally {
329+
unlink($payload);
330+
}
331+
}
332+
319333
public function testExtensionNotOk(): void
320334
{
321335
$this->validation->setRules(['avatar' => 'ext_in[avatar,xls,doc,ppt]']);
@@ -327,4 +341,72 @@ public function testExtensionImpossible(): void
327341
$this->validation->setRules(['avatar' => 'ext_in[unknown,xls,doc,ppt]']);
328342
$this->assertFalse($this->validation->run([]));
329343
}
344+
345+
public function testExtensionFailsForMismatchedClientExtension(): void
346+
{
347+
$payload = $this->createGifPayload();
348+
349+
try {
350+
$this->setUploadedAvatar($payload, 'shell.php');
351+
352+
$this->validation->setRules(['avatar' => 'ext_in[avatar,gif]']);
353+
$this->assertFalse($this->validation->run([]));
354+
} finally {
355+
unlink($payload);
356+
}
357+
}
358+
359+
public function testExtensionFailsForAllowedButMimeIncompatibleClientExtension(): void
360+
{
361+
$payload = $this->createGifPayload();
362+
363+
try {
364+
$this->setUploadedAvatar($payload, 'my-avatar.jpg');
365+
366+
$this->validation->setRules(['avatar' => 'ext_in[avatar,jpg,gif]']);
367+
$this->assertFalse($this->validation->run([]));
368+
} finally {
369+
unlink($payload);
370+
}
371+
}
372+
373+
public function testExtensionFailsForExtensionlessClientFilename(): void
374+
{
375+
$payload = $this->createGifPayload();
376+
377+
try {
378+
$this->setUploadedAvatar($payload, 'my-avatar');
379+
380+
$this->validation->setRules(['avatar' => 'ext_in[avatar,gif]']);
381+
$this->assertFalse($this->validation->run([]));
382+
} finally {
383+
unlink($payload);
384+
}
385+
}
386+
387+
private function createGifPayload(): string
388+
{
389+
$payload = tempnam(sys_get_temp_dir(), 'ci4-upload-poc-');
390+
$this->assertIsString($payload);
391+
392+
$gif = base64_decode('R0lGODlhAQABAIAAAAAAAP///ywAAAAAAQABAAACAUwAOw==', true);
393+
$this->assertIsString($gif);
394+
395+
file_put_contents($payload, $gif);
396+
397+
return $payload;
398+
}
399+
400+
private function setUploadedAvatar(string $payload, string $name): void
401+
{
402+
service('superglobals')->setFilesArray([
403+
'avatar' => [
404+
'tmp_name' => $payload,
405+
'name' => $name,
406+
'size' => filesize($payload),
407+
'type' => 'image/gif',
408+
'error' => UPLOAD_ERR_OK,
409+
],
410+
]);
411+
}
330412
}

user_guide_src/source/changelogs/v4.7.3.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ Release Date: Unreleased
1010
:local:
1111
:depth: 3
1212

13+
********
14+
SECURITY
15+
********
16+
17+
- **Validation:** The ``ext_in`` file upload validation rule now validates the
18+
client filename extension and verifies that it matches the detected MIME type.
19+
Previously, ``ext_in`` only checked the MIME-derived guessed extension, so a
20+
file with a mismatched client extension could pass validation.
21+
See the `Security advisory GHSA-2gr4-ppc7-7mhx <https://github.com/codeigniter4/CodeIgniter4/security/advisories/GHSA-2gr4-ppc7-7mhx>`_
22+
for more information.
23+
1324
********
1425
BREAKING
1526
********

user_guide_src/source/installation/upgrade_473.rst

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ upgrading. The easiest way is to re-run the install command:
3030
Breaking Changes
3131
****************
3232

33+
File Validation
34+
===============
35+
36+
The ``ext_in`` file upload validation rule now checks the client filename
37+
extension and verifies that the detected MIME type is associated with that
38+
extension. Previously, ``ext_in`` only checked the MIME-derived guessed
39+
extension.
40+
41+
This means files with no client filename extension, or files whose client
42+
filename extension does not match the detected MIME type, now fail ``ext_in``
43+
validation. If your application intentionally accepts such files, remove
44+
``ext_in`` from those validation rules and use a custom validation rule that
45+
matches your application's requirements.
46+
3347
*********************
3448
Breaking Enhancements
3549
*********************

user_guide_src/source/libraries/validation.rst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,8 +1108,10 @@ min_dims Yes Fails if the minimum width and height of an
11081108
v4.6.0.)
11091109
mime_in Yes Fails if the file's mime type is not one ``mime_in[field_name,image/png,image/jpeg]``
11101110
listed in the parameters.
1111-
ext_in Yes Fails if the file's extension is not one ``ext_in[field_name,png,jpg,gif]``
1112-
listed in the parameters.
1111+
ext_in Yes Fails if the client filename's extension is ``ext_in[field_name,png,jpg,gif]``
1112+
not one listed in the parameters, or if the
1113+
detected MIME type does not match the MIME
1114+
types associated with the extension.
11131115
is_image Yes Fails if the file cannot be determined to be ``is_image[field_name]``
11141116
an image based on the mime type.
11151117
======================= ========== ============================================= ===================================================

0 commit comments

Comments
 (0)