Skip to content

Commit b6e9a4f

Browse files
michalsnmichalsn
authored
Merge commit from fork
1 parent 20ebcf4 commit b6e9a4f

3 files changed

Lines changed: 218 additions & 2 deletions

File tree

‎system/Validation/StrictRules/FileRules.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
use CodeIgniter\Exceptions\InvalidArgumentException;
1717
use CodeIgniter\HTTP\CLIRequest;
18+
use CodeIgniter\HTTP\Files\UploadedFile;
1819
use CodeIgniter\HTTP\IncomingRequest;
1920
use CodeIgniter\HTTP\RequestInterface;
2021
use Config\Mimes;
@@ -150,6 +151,10 @@ public function is_image(?string $blank, string $params): bool
150151
if (mb_strpos($type, 'image') !== 0) {
151152
return false;
152153
}
154+
155+
if ($this->hasInvalidImageClientExtension($file)) {
156+
return false;
157+
}
153158
}
154159

155160
return true;
@@ -182,6 +187,10 @@ public function mime_in(?string $blank, string $params): bool
182187
if (! in_array($file->getMimeType(), $params, true)) {
183188
return false;
184189
}
190+
191+
if ($this->hasMismatchedClientExtension($file)) {
192+
return false;
193+
}
185194
}
186195

187196
return true;
@@ -321,4 +330,34 @@ public function min_dims(?string $blank, string $params): bool
321330

322331
return true;
323332
}
333+
334+
/**
335+
* Reject filenames with non-empty extensions that are not image types.
336+
*/
337+
private function hasInvalidImageClientExtension(UploadedFile $file): bool
338+
{
339+
$clientExtension = trim(strtolower($file->getClientExtension()), '. ');
340+
341+
if ($clientExtension === '') {
342+
return false;
343+
}
344+
345+
$type = Mimes::guessTypeFromExtension($clientExtension) ?? '';
346+
347+
return mb_strpos($type, 'image') !== 0;
348+
}
349+
350+
/**
351+
* Reject filenames with non-empty extensions that do not match the detected content.
352+
*/
353+
private function hasMismatchedClientExtension(UploadedFile $file): bool
354+
{
355+
$clientExtension = trim(strtolower($file->getClientExtension()), '. ');
356+
357+
if ($clientExtension === '') {
358+
return false;
359+
}
360+
361+
return $file->guessExtension() !== $clientExtension;
362+
}
324363
}

‎tests/system/Validation/StrictRules/FileRulesTest.php‎

Lines changed: 163 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,91 @@ public function testIsImage(): void
266266
$this->assertTrue($this->validation->run([]));
267267
}
268268

269+
public function testIsImageFailsForMismatchedClientExtension(): void
270+
{
271+
$payload = $this->createGifPayload();
272+
273+
try {
274+
$this->setUploadedAvatar($payload, 'shell.php');
275+
276+
$this->validation->setRules(['avatar' => 'is_image[avatar]']);
277+
$this->assertFalse($this->validation->run([]));
278+
} finally {
279+
unlink($payload);
280+
}
281+
}
282+
283+
public function testIsImageFailsForNonImageClientExtension(): void
284+
{
285+
$payload = $this->createGifPayload();
286+
287+
try {
288+
$this->setUploadedAvatar($payload, 'document.pdf');
289+
290+
$this->validation->setRules(['avatar' => 'is_image[avatar]']);
291+
$this->assertFalse($this->validation->run([]));
292+
} finally {
293+
unlink($payload);
294+
}
295+
}
296+
297+
public function testIsImageAllowsImageClientExtensionThatDoesNotMatchContent(): void
298+
{
299+
$payload = $this->createGifPayload();
300+
301+
try {
302+
$this->setUploadedAvatar($payload, 'my-avatar.jpg');
303+
304+
$this->validation->setRules(['avatar' => 'is_image[avatar]']);
305+
$this->assertTrue($this->validation->run([]));
306+
} finally {
307+
unlink($payload);
308+
}
309+
}
310+
311+
public function testIsImageAllowsExtensionlessClientFilename(): void
312+
{
313+
$payload = $this->createGifPayload();
314+
315+
try {
316+
$this->setUploadedAvatar($payload, 'blob');
317+
318+
$this->validation->setRules(['avatar' => 'is_image[avatar]']);
319+
$this->assertTrue($this->validation->run([]));
320+
} finally {
321+
unlink($payload);
322+
}
323+
}
324+
325+
public function testIsImageAllowsSvg(): void
326+
{
327+
$payload = $this->createSvgPayload();
328+
329+
try {
330+
$this->setUploadedAvatar($payload, 'my-avatar.svg', 'image/svg+xml');
331+
332+
$this->validation->setRules(['avatar' => 'is_image[avatar]']);
333+
$this->assertTrue($this->validation->run([]));
334+
} finally {
335+
unlink($payload);
336+
}
337+
}
338+
339+
public function testIsImageFailsForNonImageContent(): void
340+
{
341+
$payload = $this->createPhpPayload();
342+
343+
try {
344+
// An image extension is not enough; the file content must be an image too.
345+
$this->setUploadedAvatar($payload, 'fake.gif');
346+
347+
$this->validation->setRules(['avatar' => 'is_image[avatar]']);
348+
$this->assertFalse($this->validation->run([]));
349+
} finally {
350+
unlink($payload);
351+
}
352+
}
353+
269354
public function testIsntImage(): void
270355
{
271356
$_FILES['stuff'] = [
@@ -294,6 +379,62 @@ public function testMimeTypeOk(): void
294379
$this->assertTrue($this->validation->run([]));
295380
}
296381

382+
public function testMimeTypeFailsForMismatchedClientExtension(): void
383+
{
384+
$payload = $this->createGifPayload();
385+
386+
try {
387+
$this->setUploadedAvatar($payload, 'shell.php');
388+
389+
$this->validation->setRules(['avatar' => 'mime_in[avatar,image/gif]']);
390+
$this->assertFalse($this->validation->run([]));
391+
} finally {
392+
unlink($payload);
393+
}
394+
}
395+
396+
public function testMimeTypeFailsForIncompatibleClientExtension(): void
397+
{
398+
$payload = $this->createGifPayload();
399+
400+
try {
401+
$this->setUploadedAvatar($payload, 'document.pdf');
402+
403+
$this->validation->setRules(['avatar' => 'mime_in[avatar,image/gif]']);
404+
$this->assertFalse($this->validation->run([]));
405+
} finally {
406+
unlink($payload);
407+
}
408+
}
409+
410+
public function testMimeTypeFailsForAllowedClientExtensionThatDoesNotMatchContent(): void
411+
{
412+
$payload = $this->createGifPayload();
413+
414+
try {
415+
$this->setUploadedAvatar($payload, 'my-avatar.jpg');
416+
417+
$this->validation->setRules(['avatar' => 'mime_in[avatar,image/gif,image/jpeg]']);
418+
$this->assertFalse($this->validation->run([]));
419+
} finally {
420+
unlink($payload);
421+
}
422+
}
423+
424+
public function testMimeTypeAllowsExtensionlessClientFilename(): void
425+
{
426+
$payload = $this->createGifPayload();
427+
428+
try {
429+
$this->setUploadedAvatar($payload, 'blob');
430+
431+
$this->validation->setRules(['avatar' => 'mime_in[avatar,image/gif]']);
432+
$this->assertTrue($this->validation->run([]));
433+
} finally {
434+
unlink($payload);
435+
}
436+
}
437+
297438
public function testMimeTypeNotOk(): void
298439
{
299440
$this->validation->setRules([
@@ -397,14 +538,34 @@ private function createGifPayload(): string
397538
return $payload;
398539
}
399540

400-
private function setUploadedAvatar(string $payload, string $name): void
541+
private function createPhpPayload(): string
542+
{
543+
$payload = tempnam(sys_get_temp_dir(), 'ci4-upload-poc-');
544+
$this->assertIsString($payload);
545+
546+
file_put_contents($payload, "<?php echo 'pwned'; ?>\n");
547+
548+
return $payload;
549+
}
550+
551+
private function createSvgPayload(): string
552+
{
553+
$payload = tempnam(sys_get_temp_dir(), 'ci4-upload-poc-');
554+
$this->assertIsString($payload);
555+
556+
file_put_contents($payload, '<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"></svg>');
557+
558+
return $payload;
559+
}
560+
561+
private function setUploadedAvatar(string $payload, string $name, string $clientMimeType = 'image/gif'): void
401562
{
402563
service('superglobals')->setFilesArray([
403564
'avatar' => [
404565
'tmp_name' => $payload,
405566
'name' => $name,
406567
'size' => filesize($payload),
407-
'type' => 'image/gif',
568+
'type' => $clientMimeType,
408569
'error' => UPLOAD_ERR_OK,
409570
],
410571
]);

‎user_guide_src/source/changelogs/v4.7.4.rst‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,22 @@ SECURITY
3939
See the `Security advisory GHSA-hhmc-q9hp-r662 <https://github.com/codeigniter4/CodeIgniter4/security/advisories/GHSA-hhmc-q9hp-r662>`_
4040
for more information.
4141

42+
- **Validation:** The ``is_image`` and ``mime_in`` file upload validation rules
43+
now also verify non-empty client filename extensions. Previously, these rules
44+
classified an upload solely by its content-derived MIME type, so a file with a
45+
dangerous client extension (for example, a ``.php`` file prepended with image
46+
magic bytes) could pass validation while keeping its original extension on
47+
disk.
48+
See the `Security advisory GHSA-mmj4-63m4-r6h5 <https://github.com/codeigniter4/CodeIgniter4/security/advisories/GHSA-mmj4-63m4-r6h5>`_
49+
for more information.
50+
51+
The ``is_image`` rule now rejects uploads when a non-empty client filename
52+
extension is not an image extension. The ``mime_in`` rule now rejects uploads
53+
when a non-empty client filename extension does not match the detected file
54+
content, matching the same extension/content agreement used by ``ext_in``.
55+
Files uploaded without any extension (such as JavaScript ``Blob`` uploads)
56+
are still accepted, since the rules validate the file content.
57+
4258
********
4359
BREAKING
4460
********

0 commit comments

Comments
 (0)