From f4deb0d1c89d070ad69bda28eb811db5ea5bcc32 Mon Sep 17 00:00:00 2001 From: Shivanath Devinarayanan Date: Fri, 23 Oct 2026 01:56:31 +0400 Subject: [PATCH] fix(files): derive the stored extension from the detected file type Uploads were stored under the client's original extension, while UploadFileRequest validates only the type detected from the contents. A file whose contents pass that check could therefore be stored as .php or .phtml, for example a PNG with PHP appended. Add File::storageExtensionForUpload(). It keeps the client's extension when it matches the detected type or is a plain data format that detection reports loosely (CSV, XLSX, JSON, certificates and so on), otherwise takes the extension of the detected type, falling back to "bin", and never returns an extension a web server may execute. Use it for multipart uploads (randomFileNameFromRequest) and for FileResolverService uploads. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Models/File.php | 41 +++++++++++++++++++++++++- src/Services/FileResolverService.php | 2 +- tests/Unit/Http/FileControllerTest.php | 17 +++++++++-- tests/Unit/Models/FileModelTest.php | 25 ++++++++++++++++ 4 files changed, 81 insertions(+), 4 deletions(-) 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'); +});