diff --git a/src/Models/File.php b/src/Models/File.php index 1eaa37e3..19b32734 100644 --- a/src/Models/File.php +++ b/src/Models/File.php @@ -32,6 +32,21 @@ class File extends Model use LogsActivity; use SendsWebhooks; + /** + * Extensions an uploaded file is never stored under, because a web server may execute them. + */ + public const EXECUTABLE_UPLOAD_EXTENSIONS = [ + 'php', 'php3', 'php4', 'php5', 'php6', 'php7', 'php8', 'phtml', 'phar', 'pht', 'phps', 'shtml', 'htaccess', 'cgi', + ]; + + /** + * Data formats whose extension is kept even when content detection reports a looser type. + */ + public const DATA_UPLOAD_EXTENSIONS = [ + 'csv', 'tsv', 'txt', 'json', 'xls', 'xlsx', 'doc', 'docx', 'ppt', 'pptx', 'pdf', + 'zip', 'tar', 'gz', 'tgz', 'bz2', 'xz', 'p12', 'pfx', 'pem', 'crt', 'cer', 'der', 'p7b', 'p7c', + ]; + /** * The type of public Id to generate. * @@ -608,7 +623,7 @@ public static function randomFileNameFromRequest($request, string $filename = 'f /** @var \Illuminate\Http\File|Symfony\Component\HttpFoundation\File\File|\Symfony\Component\HttpFoundation\File\UploadedFile $file */ if ($request->hasFile($filename)) { $file = $request->file($filename); - $extension = $extension ?? strtolower($file->getClientOriginalExtension()); + $extension = $extension ?? static::storageExtensionForUpload($file); $extension = Str::startsWith($extension, '.') ? $extension : '.' . $extension; $sqids = new \Sqids\Sqids(); @@ -618,6 +633,30 @@ public static function randomFileNameFromRequest($request, string $filename = 'f return static::randomFileName($extension); } + /** + * The extension to store an uploaded file under. + * + * The client's extension is kept when it matches the type detected from the file's + * contents, or when it is a plain data format that detection often reports loosely + * (a CSV as text/plain, an XLSX as application/zip). Otherwise the extension is taken + * from the detected type, falling back to "bin". Extensions a web server may execute + * are never used. + */ + public static function storageExtensionForUpload(UploadedFile $file): string + { + $clientExtension = strtolower($file->getClientOriginalExtension()); + $detectedType = $file->getMimeType(); + $typeExtensions = $detectedType ? \Symfony\Component\Mime\MimeTypes::getDefault()->getExtensions($detectedType) : []; + + if ($clientExtension !== '' && (in_array($clientExtension, $typeExtensions, true) || in_array($clientExtension, static::DATA_UPLOAD_EXTENSIONS, true))) { + $extension = $clientExtension; + } else { + $extension = strtolower($file->guessExtension() ?? 'bin'); + } + + return in_array($extension, static::EXECUTABLE_UPLOAD_EXTENSIONS, true) ? 'bin' : $extension; + } + /** * Retrieves a collection of files based on UUIDs provided via a request. * diff --git a/src/Services/FileResolverService.php b/src/Services/FileResolverService.php index 3382b304..0ca17c69 100644 --- a/src/Services/FileResolverService.php +++ b/src/Services/FileResolverService.php @@ -95,7 +95,7 @@ protected function resolveFromUpload(UploadedFile $upload, string $path, ?string $disk = $disk ?? config('filesystems.default'); // Generate a unique filename to prevent collisions - $extension = $upload->getClientOriginalExtension(); + $extension = File::storageExtensionForUpload($upload); $fileName = File::randomFileName($extension); // Ensure path doesn't have trailing slash diff --git a/tests/Unit/Http/FileControllerTest.php b/tests/Unit/Http/FileControllerTest.php index 0f0d91a5..109fa911 100644 --- a/tests/Unit/Http/FileControllerTest.php +++ b/tests/Unit/Http/FileControllerTest.php @@ -429,11 +429,11 @@ function public_file_controller_failing_upload_request(array $input = []): Uploa return UploadFileRequest::create('/v1/files', 'POST', $input, [], ['file' => $file]); } -function file_controller_upload_request(array $input = [], string $contents = 'uploaded body'): UploadFileRequest +function file_controller_upload_request(array $input = [], string $contents = 'uploaded body', string $originalName = 'manifest.txt'): UploadFileRequest { $path = tempnam(sys_get_temp_dir(), 'fleetbase-upload-'); file_put_contents($path, $contents); - $file = new UploadedFile($path, 'manifest.txt', 'text/plain', null, true); + $file = new UploadedFile($path, $originalName, 'text/plain', null, true); return UploadFileRequest::create('/int/v1/files', 'POST', $input, [], ['file' => $file]); } @@ -510,6 +510,19 @@ function file_controller_download_request(array $query = []): DownloadFileReques ->and(Storage::disk('testing')->get('uploads/documents/manifest.txt'))->toBe('plain text body'); }); +test('file controller stores uploads under the extension of their detected type, never a script extension', function () { + $capsule = file_controller_fixtures(); + $png = base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8Xw8AAoMBgBzq7oQAAAAASUVORK5CYII='); + + $response = file_controller()->upload(file_controller_upload_request(['path' => 'uploads'], $png . '', 'shell.php')); + $record = $capsule->getConnection('mysql')->table('files')->first(); + + expect($response->getStatusCode())->toBe(200) + ->and($record->path)->toStartWith('uploads/') + ->and($record->path)->toEndWith('.png') + ->and(Storage::disk('testing')->get($record->path))->toBe($png . ''); +}); + test('file controller uploads multipart files with generated storage path and subject metadata', function () { $capsule = file_controller_fixtures(); diff --git a/tests/Unit/Models/FileModelTest.php b/tests/Unit/Models/FileModelTest.php index 3e416eac..2ea45c95 100644 --- a/tests/Unit/Models/FileModelTest.php +++ b/tests/Unit/Models/FileModelTest.php @@ -569,3 +569,28 @@ function bind_file_model_filesystem(array $config = []): FileModelFilesystemFake ->and($gif)->toEndWith('.gif') ->and($png)->not->toBe($jpg); }); + +it('stores uploads under an extension that matches their contents and never a script extension', function () { + $png = base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8Xw8AAoMBgBzq7oQAAAAASUVORK5CYII='); + $upload = function (string $contents, string $name): UploadedFile { + $path = tempnam(sys_get_temp_dir(), 'fleetbase-upload-'); + file_put_contents($path, $contents); + + return new UploadedFile($path, $name, 'application/octet-stream', null, true); + }; + + expect(File::storageExtensionForUpload($upload($png, 'photo.PNG')))->toBe('png') + ->and(File::storageExtensionForUpload($upload($png, 'photo.jpg')))->toBe('png') + ->and(File::storageExtensionForUpload($upload($png . '', 'shell.php')))->toBe('png') + ->and(File::storageExtensionForUpload($upload($png, 'shell.phtml')))->toBe('png') + ->and(File::storageExtensionForUpload($upload($png, 'no-extension')))->toBe('png') + ->and(File::storageExtensionForUpload($upload("id,name\n1,Alpha\n2,Beta\n", 'orders.csv')))->toBe('csv') + ->and(File::storageExtensionForUpload($upload('{"a":1}', 'config.json')))->toBe('json') + ->and(File::storageExtensionForUpload($upload('plain notes', 'notes.phar')))->toBe('txt') + ->and(File::storageExtensionForUpload($upload(str_repeat("\x00\x9f\x01\xfe", 16), 'blob.php5')))->toBe('bin'); + + $request = Request::create('/int/v1/files/upload', 'POST', [], [], ['file' => $upload($png . '', 'shell.php')]); + + expect(File::randomFileNameFromRequest($request))->toEndWith('.png') + ->and(File::randomFileNameFromRequest($request, 'file', 'webp'))->toEndWith('.webp'); +});