diff --git a/composer.json b/composer.json index b9085533..5a56d953 100644 --- a/composer.json +++ b/composer.json @@ -33,6 +33,7 @@ "symfony/cache": "^8.1", "symfony/console": "^8.1", "symfony/mailer": "^8.1", + "symfony/mime": "^8.1", "symfony/process": "^8.1", "symfony/var-dumper": "^8.1" }, diff --git a/src/Phaseolies/Http/Support/ValidationRules.php b/src/Phaseolies/Http/Support/ValidationRules.php index 8ed11464..ccc67d3d 100644 --- a/src/Phaseolies/Http/Support/ValidationRules.php +++ b/src/Phaseolies/Http/Support/ValidationRules.php @@ -3,6 +3,7 @@ namespace Phaseolies\Http\Support; use Phaseolies\Translation\Translator; +use Symfony\Component\Mime\MimeTypes; trait ValidationRules { @@ -691,9 +692,7 @@ protected function validateFile(string $fieldName, string $rule, mixed $ruleValu break; case 'mimes': - $allowedTypes = explode(',', $ruleValue); - $fileExtension = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION)); - if (!in_array($fileExtension, $allowedTypes)) { + if (!$this->fileMatchesAllowedExtensions($file['tmp_name'], $ruleValue)) { return $this->getErrorMessage('file.mimes', $fieldName, [ ':values' => $ruleValue, 'values' => $ruleValue @@ -750,6 +749,40 @@ protected function validateFile(string $fieldName, string $rule, mixed $ruleValu return null; } + /** + * Checks whether a file's real content matches one of the extensions allowed by a "mimes" rule. + * + * @param string $tmpPath + * @param string $allowedExtensionsList + * @return bool + */ + protected function fileMatchesAllowedExtensions(string $tmpPath, string $allowedExtensionsList): bool + { + $mimeTypes = MimeTypes::getDefault(); + + try { + $detectedMime = $mimeTypes->guessMimeType($tmpPath); + } catch (\LogicException) { + // No MIME guesser available (e.g. the fileinfo extension is + // missing): fail closed rather than trust unverified input. + return false; + } + + if ($detectedMime === null) { + return false; + } + + foreach (explode(',', $allowedExtensionsList) as $extension) { + $expectedMimes = $mimeTypes->getMimeTypes(strtolower(trim($extension))); + + if (in_array($detectedMime, $expectedMimes, true)) { + return true; + } + } + + return false; + } + /** * Parse the dimensions rule value. * diff --git a/tests/Validation/FileValidationRuleTest.php b/tests/Validation/FileValidationRuleTest.php new file mode 100644 index 00000000..283990ba --- /dev/null +++ b/tests/Validation/FileValidationRuleTest.php @@ -0,0 +1,131 @@ +>endobj\ntrailer<<>>\n%%EOF"; + + private string $tmpDir; + + protected function setUp(): void + { + parent::setUp(); + + Container::setInstance(new MockContainer()); + $container = new Container(); + $container->bind('translator', function () { + $loader = $this->createMock(FileLoader::class); + return new Translator($loader, 'en'); + }); + + $this->tmpDir = sys_get_temp_dir() . '/doppar_mimes_test_' . uniqid(); + mkdir($this->tmpDir, 0755, true); + + $_FILES = []; + } + + protected function tearDown(): void + { + $_FILES = []; + + foreach (glob($this->tmpDir . '/*') ?: [] as $file) { + unlink($file); + } + rmdir($this->tmpDir); + + parent::tearDown(); + } + + private function registerUploadedFile( + string $field, + string $contents, + string $claimedName = 'upload', + string $claimedType = 'application/octet-stream', + int $error = UPLOAD_ERR_OK + ): void { + $path = $this->tmpDir . '/' . bin2hex(random_bytes(8)); + file_put_contents($path, $contents); + + $_FILES[$field] = [ + 'name' => $claimedName, + 'type' => $claimedType, + 'tmp_name' => $path, + 'error' => $error, + 'size' => filesize($path), + ]; + } + + private function passes(array $rules): bool + { + return (new Sanitizer([], $rules))->validate(); + } + + public function testMimesRejectsScriptPayloadDisguisedWithAnAllowedExtension(): void + { + $this->registerUploadedFile('file', '', 'shell.pdf', 'application/pdf'); + + $this->assertFalse($this->passes(['file' => 'mimes:pdf,doc'])); + } + + public function testMimesRejectsScriptPayloadEvenWithSpoofedClientContentType(): void + { + // The client Content-Type header is attacker-controlled; this proves + // the rule doesn't fall back to trusting it. + $this->registerUploadedFile('file', '', 'shell.pdf', 'application/pdf'); + + $this->assertFalse($this->passes(['file' => 'mimes:pdf'])); + } + + public function testMimesAcceptsRealContentMatchingAnAllowedExtension(): void + { + $this->registerUploadedFile('file', self::PDF_BYTES, 'contract.pdf', 'application/pdf'); + + $this->assertTrue($this->passes(['file' => 'mimes:pdf,doc'])); + } + + public function testMimesRejectsRealContentNotMatchingAnyAllowedExtension(): void + { + $this->registerUploadedFile('file', 'just plain text', 'notes.txt', 'text/plain'); + + $this->assertFalse($this->passes(['file' => 'mimes:pdf,doc'])); + } + + public function testMimesAcceptsRealImageContentRegardlessOfClaimedContentType(): void + { + $pngBytes = base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII='); + $this->registerUploadedFile('file', $pngBytes, 'photo.jpg', 'application/octet-stream'); + + $this->assertTrue($this->passes(['file' => 'mimes:jpg,jpeg,png'])); + } + + public function testMimesIsCaseInsensitiveForTheAllowedExtensionList(): void + { + $this->registerUploadedFile('file', self::PDF_BYTES, 'contract.PDF', 'application/pdf'); + + $this->assertTrue($this->passes(['file' => 'mimes:PDF'])); + } + + public function testMimesCombinesWithRequiredAndRejectsWhenNoFileIsUploaded(): void + { + $_FILES = []; + + $this->assertFalse($this->passes(['file' => 'required|mimes:pdf'])); + } + + public function testImageRuleStillRejectsScriptPayloadDisguisedAsImage(): void + { + $this->registerUploadedFile('file', '', 'avatar.jpg', 'image/jpeg'); + + $this->assertFalse($this->passes(['file' => 'image'])); + } +}