diff --git a/ProcessMaker/Http/Controllers/Api/ProcessRequestFileController.php b/ProcessMaker/Http/Controllers/Api/ProcessRequestFileController.php index 2968524338..5e38a77d41 100644 --- a/ProcessMaker/Http/Controllers/Api/ProcessRequestFileController.php +++ b/ProcessMaker/Http/Controllers/Api/ProcessRequestFileController.php @@ -289,14 +289,14 @@ public function store(Request $laravel_request, FileReceiver $receiver, ProcessR } catch (FileIsTooBig $e) { return response()->json([ 'errors' => [ - 'file' => ['file may not be greater than ' . (config('media-library.max_file_size') / 1024) . ' kilobytes'] - ] + 'file' => ['file may not be greater than ' . (config('media-library.max_file_size') / 1024) . ' kilobytes'], + ], ], 422); } catch (Exception $e) { return response()->json([ 'errors' => [ - 'message' => $e->getMessage() - ] + 'message' => $e->getMessage(), + ], ], 500); } } @@ -440,15 +440,110 @@ public function destroy(Request $laravel_request, ProcessRequest $request, $file return response([], 204); } + /** + * Validate uploaded file for security and type restrictions + * + * @param UploadedFile $file + * @param array $errors + * @return array + */ private function validateFile(UploadedFile $file, &$errors) { - if (strtolower($file->getClientOriginalExtension() === 'pdf')) { + // Explicitly reject archive files for security + $this->rejectArchiveFiles($file, $errors); + + // Validate file extension if enabled + if (config('files.enable_extension_validation', true)) { + $this->validateFileExtension($file, $errors); + } + + // Validate MIME type vs extension if enabled + if (config('files.enable_mime_validation', true)) { + $this->validateExtensionMimeTypeMatch($file, $errors); + } + + // Validate specific file types (e.g., PDF for JavaScript content) + if (strtolower($file->getClientOriginalExtension()) === 'pdf') { $this->validatePDFFile($file, $errors); } return $errors; } + /** + * Explicitly reject archive files for security reasons + * + * @param UploadedFile $file + * @param array $errors + * @return void + */ + private function rejectArchiveFiles(UploadedFile $file, &$errors) + { + $dangerousExtensions = config('files.dangerous_extensions'); + + $fileExtension = strtolower($file->getClientOriginalExtension()); + + if (in_array($fileExtension, $dangerousExtensions)) { + $errors['message'] = __('Uploaded file type is not allowed'); + + return; + } + + // Also check MIME types for archive files + $dangerousMimeTypes = config('files.dangerous_mime_types'); + + $fileMimeType = $file->getMimeType(); + + if (in_array($fileMimeType, $dangerousMimeTypes)) { + $errors['message'] = __('Uploaded mime file type is not allowed'); + } + } + + /** + * Validate that file extension matches the MIME type + * + * @param UploadedFile $file + * @param array $errors + * @return void + */ + private function validateExtensionMimeTypeMatch(UploadedFile $file, &$errors) + { + $fileExtension = strtolower($file->getClientOriginalExtension()); + $fileMimeType = $file->getMimeType(); + + // Get extension to MIME type mapping from configuration + $extensionMimeMap = config('files.extension_mime_map'); + + // Check if extension exists in our map + if (!isset($extensionMimeMap[$fileExtension])) { + $errors['message'] = __('File extension not allowed'); + + return; + } + + // Check if MIME type matches any of the expected types for this extension + if (!in_array($fileMimeType, $extensionMimeMap[$fileExtension])) { + $errors['message'] = __('The file extension does not match the actual file content'); + } + } + + /** + * Validate file extension against allowed extensions + * + * @param UploadedFile $file + * @param array $errors + * @return void + */ + private function validateFileExtension(UploadedFile $file, &$errors) + { + $allowedExtensions = config('files.allowed_extensions'); + $fileExtension = strtolower($file->getClientOriginalExtension()); + + if (!in_array($fileExtension, $allowedExtensions)) { + $errors['message'] = __('File extension not allowed'); + } + } + private function validatePDFFile(UploadedFile $file, &$errors) { $text = $file->get(); diff --git a/config/files.php b/config/files.php new file mode 100644 index 0000000000..33dad27d84 --- /dev/null +++ b/config/files.php @@ -0,0 +1,138 @@ + [ + // Documents + 'pdf', 'doc', 'docx', 'xls', 'xlsx', 'ppt', 'pptx', + 'txt', 'csv', + + // Images + 'jpg', 'jpeg', 'png', 'gif', 'svg', + + // Audio + 'mp3', + + // Video + 'mp4', + ], + /* + |-------------------------------------------------------------------------- + | Extension to MIME Type Mapping + |-------------------------------------------------------------------------- + | + | An associative array that maps each allowed file extension to one or more + | corresponding MIME types. This provides a strong validation to ensure that + | a file's content type (MIME type) matches its declared extension, + | preventing malicious files (like a script disguised as an image) from being uploaded. + | + */ + 'extension_mime_map' => [ + // Documents + 'pdf' => ['application/pdf'], + 'doc' => ['application/msword'], + 'docx' => ['application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + 'xls' => ['application/vnd.ms-excel'], + 'xlsx' => ['application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + 'ppt' => ['application/vnd.ms-powerpoint'], + 'pptx' => ['application/vnd.openxmlformats-officedocument.presentationml.presentation'], + 'txt' => ['text/plain'], + 'csv' => ['text/csv', 'application/csv'], + + // Audio + 'jpg' => ['image/jpeg'], + 'jpeg' => ['image/jpeg'], + 'png' => ['image/png'], + 'gif' => ['image/gif'], + 'svg' => ['image/svg+xml'], + + // Audio + 'mp3' => ['audio/mpeg'], + + // Video + 'mp4' => ['video/mp4'], + ], + + /* + |-------------------------------------------------------------------------- + | Enable MIME Type Validation + |-------------------------------------------------------------------------- + | + | Whether to enable MIME type validation against allowed_mime_types list + | AND validate that MIME type corresponds to file extension using extension_mime_map. + | This provides comprehensive validation to prevent malicious files. + | Recommended to keep this enabled for security. + | + */ + 'enable_mime_validation' => env('ENABLE_MIME_VALIDATION', true), + + /* + |-------------------------------------------------------------------------- + | Enable Extension Validation + |-------------------------------------------------------------------------- + | + | Whether to enable basic file extension validation against allowed_extensions list. + | This validates that the file extension is in the allowed list. + | Recommended to keep this enabled for security. + | + */ + 'enable_extension_validation' => env('ENABLE_EXTENSION_VALIDATION', true), + + /* + |-------------------------------------------------------------------------- + | Security Dangerous File Extensions + |-------------------------------------------------------------------------- + | + | Archive formats (.zip, .rar, .tar, .7z, .gz, etc.) are explicitly + | NOT allowed for security reasons. These file types can contain + | malicious content and are blocked by default. + | + */ + 'dangerous_extensions' => [ + 'zip', 'rar', '7z', 'tar', 'gz', 'bz2', 'xz', 'lzma', + 'cab', 'ar', 'iso', 'dmg', 'pkg', 'deb', 'rpm', + ], + + /* + |-------------------------------------------------------------------------- + | Security Dangerous MIME Types + |-------------------------------------------------------------------------- + | + | A list of MIME types associated with archives and executables. + | This provides an additional layer of security to prevent the upload of + | compressed files or other potentially dangerous content, even if their + | file extension has been tampered with. + | + */ + 'dangerous_mime_types' => [ + 'application/zip', + 'application/x-rar-compressed', + 'application/x-7z-compressed', + 'application/x-tar', + 'application/gzip', + 'application/x-bzip2', + 'application/x-xz', + 'application/x-lzma', + 'application/vnd.ms-cab-compressed', + 'application/x-iso9660-image', + ], +]; diff --git a/resources/lang/en.json b/resources/lang/en.json index a3de137b81..c4e384c556 100644 --- a/resources/lang/en.json +++ b/resources/lang/en.json @@ -944,6 +944,7 @@ "File Access": "File Access", "File crt": "File crt", "File Download": "File Download", + "File extension not allowed": "File extension not allowed", "File ID does not exist": "File ID does not exist", "File key": "File key", "File Manager": "File Manager", @@ -2198,6 +2199,7 @@ "The field unter validation must be after or equal to the given field.": "The field unter validation must be after or equal to the given field.", "The field unter validation must be before or equal to the given field.": "The field unter validation must be before or equal to the given field.", "The field unter validation must be before the given date.": "The field unter validation must be before the given date.", + "The file extension does not match the actual file content": "The file extension does not match the actual file content", "The file is processing. You may continue working while the log file compiles.": "The file is processing. You may continue working while the log file compiles.", "The file you are importing was made with an older version of ProcessMaker. Advanced import is not available. All assets will be copied.": "The file you are importing was made with an older version of ProcessMaker. Advanced import is not available. All assets will be copied.", "The following items should be configured to ensure your process is functional.": "The following items should be configured to ensure your process is functional.", @@ -2443,6 +2445,8 @@ "Upload": "Upload", "Uploaded By": "Uploaded By", "Uploaded": "Uploaded", + "Uploaded file type is not allowed.": "Uploaded file type is not allowed.", + "Uploaded mime file type is not allowed": "Uploaded mime file type is not allowed", "Uploading...": "Uploading...", "Uppercase characters": "Uppercase characters", "URI": "URI", diff --git a/tests/unit/ProcessRequestFileControllerValidationTest.php b/tests/unit/ProcessRequestFileControllerValidationTest.php new file mode 100644 index 0000000000..5435fd84be --- /dev/null +++ b/tests/unit/ProcessRequestFileControllerValidationTest.php @@ -0,0 +1,424 @@ +user = User::factory()->create(); + $this->processRequest = ProcessRequest::factory()->create(); + + // Create controller instance + $this->controller = new ProcessRequestFileController(); + + // Set up test configuration + Config::set('files.enable_extension_validation', true); + Config::set('files.enable_mime_validation', true); + Config::set('files.allowed_extensions', ['pdf', 'doc', 'docx', 'txt', 'jpg', 'png']); + Config::set('files.dangerous_extensions', ['zip', 'rar', '7z', 'tar']); + Config::set('files.dangerous_mime_types', [ + 'application/zip', + 'application/x-rar-compressed', + 'application/x-7z-compressed', + 'application/x-tar', + ]); + Config::set('files.extension_mime_map', [ + 'pdf' => ['application/pdf'], + 'doc' => ['application/msword'], + 'docx' => ['application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + 'txt' => ['text/plain'], + 'jpg' => ['image/jpeg'], + 'png' => ['image/png'], + ]); + } + + /** + * Helper method to invoke the private validateFile method + */ + private function invokeValidateFile(UploadedFile $file, &$errors) + { + $reflection = new ReflectionClass($this->controller); + $method = $reflection->getMethod('validateFile'); + $method->setAccessible(true); + + // Use invokeArgs with an array that will be passed by reference + $args = [$file, &$errors]; + + return $method->invokeArgs($this->controller, $args); + } + + /** + * Helper method to invoke the private rejectArchiveFiles method + */ + private function invokeRejectArchiveFiles(UploadedFile $file, &$errors) + { + $reflection = new ReflectionClass($this->controller); + $method = $reflection->getMethod('rejectArchiveFiles'); + $method->setAccessible(true); + + // Use invokeArgs with an array that will be passed by reference + $args = [$file, &$errors]; + + return $method->invokeArgs($this->controller, $args); + } + + /** + * Helper method to invoke the private validateFileExtension method + */ + private function invokeValidateFileExtension(UploadedFile $file, &$errors) + { + $reflection = new ReflectionClass($this->controller); + $method = $reflection->getMethod('validateFileExtension'); + $method->setAccessible(true); + + // Use invokeArgs with an array that will be passed by reference + $args = [$file, &$errors]; + + return $method->invokeArgs($this->controller, $args); + } + + /** + * Helper method to invoke the private validateExtensionMimeTypeMatch method + */ + private function invokeValidateExtensionMimeTypeMatch(UploadedFile $file, &$errors) + { + $reflection = new ReflectionClass($this->controller); + $method = $reflection->getMethod('validateExtensionMimeTypeMatch'); + $method->setAccessible(true); + + // Use invokeArgs with an array that will be passed by reference + $args = [$file, &$errors]; + + return $method->invokeArgs($this->controller, $args); + } + + /** + * Helper method to invoke the private validatePDFFile method + */ + private function invokeValidatePDFFile(UploadedFile $file, &$errors) + { + $reflection = new ReflectionClass($this->controller); + $method = $reflection->getMethod('validatePDFFile'); + $method->setAccessible(true); + + // Use invokeArgs with an array that will be passed by reference + $args = [$file, &$errors]; + + return $method->invokeArgs($this->controller, $args); + } + + /** + * Test the main validateFile method with valid file + */ + public function testValidateFileWithValidFile() + { + $file = UploadedFile::fake()->create('document.pdf', 100, 'application/pdf'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + $this->assertEmpty($errors, 'Valid PDF file should not generate errors'); + } + + /** + * Test validateFile with archive file (should be rejected) + */ + public function testValidateFileWithArchiveFile() + { + $file = UploadedFile::fake()->create('archive.zip', 100, 'application/zip'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + $this->assertNotEmpty($errors, 'Archive file should generate errors'); + $this->assertStringContainsString('File extension not allowed', $errors['message']); + } + + /** + * Test validateFile with extension not in allowed list + */ + public function testValidateFileWithUnallowedExtension() + { + $file = UploadedFile::fake()->create('document.xyz', 100, 'application/octet-stream'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + $this->assertNotEmpty($errors, 'Unallowed extension should generate errors'); + $this->assertStringContainsString('File extension not allowed', $errors['message']); + } + + /** + * Test validateFile with extension vs MIME type mismatch + */ + public function testValidateFileWithExtensionMimeTypeMismatch() + { + $file = UploadedFile::fake()->create('document.pdf', 100, 'text/plain'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + $this->assertNotEmpty($errors, 'Extension vs MIME type mismatch should generate errors'); + $this->assertStringContainsString('The file extension does not match the actual file content', $errors['message']); + } + + /** + * Test validateFile with dangerous PDF content + */ + public function testValidateFileWithDangerousPDFContent() + { + // Create a PDF file with JavaScript content + $pdfContent = '%PDF-1.4 +1 0 obj +<< +/Type /Catalog +/Pages 2 0 R +>> +endobj +2 0 obj +<< +/Type /Pages +/Kids [3 0 R] +/Count 1 +>> +endobj +3 0 obj +<< +/Type /Page +/Parent 2 0 R +/MediaBox [0 0 612 792] +/Contents 4 0 R +>> +endobj +4 0 obj +<< +/Length 44 +>> +stream +/JavaScript +<< +/S /JavaScript +>> +endstream +endobj +xref +0 5 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +0000000204 00000 n +trailer +<< +/Size 5 +/Root 1 0 R +>> +startxref +264 +%%EOF'; + + $file = UploadedFile::fake()->createWithContent('dangerous.pdf', $pdfContent); + $file->mimeType('application/pdf'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + $this->assertNotEmpty($errors, 'Dangerous PDF content should generate errors'); + $this->assertStringContainsString('Dangerous PDF file content', implode(', ', $errors)); + } + + /** + * Test validateFile with extension validation disabled + */ + public function testDisableMimeAndExtensionValidation() + { + Config::set('files.enable_extension_validation', false); + Config::set('files.enable_mime_validation', false); + + $file = UploadedFile::fake()->create('document.xyz', 100, 'application/octet-stream'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + // Should not have extension validation errors, but should still have MIME type validation + $extensionErrors = array_filter($errors, function ($error) { + return strpos($error, 'File extension not allowed') !== false; + }); + $this->assertEmpty($extensionErrors, 'Extension validation should be disabled'); + } + + /** + * Test validateFile with extension validation disabled + */ + public function testValidateFileWithExtensionValidationDisabled() + { + Config::set('files.enable_extension_validation', false); + + $file = UploadedFile::fake()->create('document.xyz', 100, 'application/octet-stream'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + // Should not have extension validation errors, but should still have MIME type validation + $extensionErrors = array_filter($errors, function ($error) { + return strpos($error, 'File extension not allowed') !== false; + }); + $this->assertNotEmpty($extensionErrors, 'Extension validation should be disabled but the MIME type should still be validated'); + } + + /** + * Test validateFile with MIME validation disabled + */ + public function testValidateFileWithMimeValidationDisabled() + { + Config::set('files.enable_mime_validation', false); + + $file = UploadedFile::fake()->create('document.pdf', 100, 'text/plain'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + // Should not have MIME type validation errors + $mimeErrors = array_filter($errors, function ($error) { + return strpos($error, 'The file extension does not match the actual file content') !== false; + }); + + $this->assertEmpty($mimeErrors, 'MIME type validation should be disabled'); + } + + /** + * Test rejectArchiveFiles method specifically + */ + public function testRejectArchiveFiles() + { + $file = UploadedFile::fake()->create('archive.zip', 100, 'application/zip'); + + $errors = []; + $this->invokeRejectArchiveFiles($file, $errors); + + $this->assertNotEmpty($errors, 'Archive file should be rejected'); + $this->assertStringContainsString('Uploaded file type is not allowed', $errors['message']); + } + + /** + * Test validateFileExtension method specifically + */ + public function testValidateFileExtension() + { + $file = UploadedFile::fake()->create('document.xyz', 100, 'application/octet-stream'); + + $errors = []; + $this->invokeValidateFileExtension($file, $errors); + + $this->assertNotEmpty($errors, 'Unallowed extension should generate errors'); + $this->assertStringContainsString('File extension not allowed', $errors['message']); + } + + /** + * Test validateExtensionMimeTypeMatch method specifically + */ + public function testValidateExtensionMimeTypeMatch() + { + $file = UploadedFile::fake()->create('document.pdf', 100, 'text/plain'); + + $errors = []; + $this->invokeValidateExtensionMimeTypeMatch($file, $errors); + + $this->assertNotEmpty($errors, 'Extension vs MIME type mismatch should generate errors'); + $this->assertStringContainsString('The file extension does not match the actual file content', $errors['message']); + } + + /** + * Test validatePDFFile method specifically + */ + public function testValidatePDFFile() + { + $pdfContent = '%PDF-1.4 +1 0 obj +<< +/Type /Catalog +/Pages 2 0 R +>> +endobj +2 0 obj +<< +/Type /Pages +/Kids [3 0 R] +/Count 1 +>> +endobj +3 0 obj +<< +/Type /Page +/Parent 2 0 R +/MediaBox [0 0 612 792] +/Contents 4 0 R +>> +endobj +4 0 obj +<< +/Length 44 +>> +stream +/JavaScript +<< +/S /JavaScript +>> +endstream +endobj +xref +0 5 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +0000000204 00000 n +trailer +<< +/Size 5 +/Root 1 0 R +>> +startxref +264 +%%EOF'; + + $file = UploadedFile::fake()->createWithContent('dangerous.pdf', $pdfContent); + + $errors = []; + $this->invokeValidatePDFFile($file, $errors); + + $this->assertNotEmpty($errors, 'Dangerous PDF content should generate errors'); + $this->assertStringContainsString('Dangerous PDF file content', implode(', ', $errors)); + } + + /** + * Test multiple validation errors at once + */ + public function testValidateFileWithMultipleErrors() + { + // Create a file that violates multiple rules + $file = UploadedFile::fake()->create('archive.xyz', 100, 'application/octet-stream'); + + $errors = []; + $this->invokeValidateFile($file, $errors); + + $this->assertEquals(1, count($errors), 'Should have multiple validation errors'); + } +}