-
Notifications
You must be signed in to change notification settings - Fork 243
It is possible to upload dangerous files from Web Entry #8573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…and type restrictions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| // Validate MIME type vs extension if enabled | ||
| if (config('files.enable_mime_validation', true)) { | ||
| $this->validateExtensionMimeTypeMatch($file, $errors); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: File Validation Defaults Cause Security Concerns
The file validation trait has inconsistent default config values. enable_dangerous_validation is disabled by default when not configured, but enable_extension_validation and enable_mime_validation default to true. This changes the previous opt-in behavior, potentially enabling validations unexpectedly and creating a security risk by silently skipping critical dangerous file checks.
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Inconsistent Error Handling in Validation Methods
The validatePDFFile method adds errors using an indexed array append ($errors[]), which differs from other validation methods in this trait that use an associative key ($errors['message']). This inconsistency can lead to unexpected error array structures, potentially breaking downstream error handling that expects a consistent 'message' key.
| private function validateFile(UploadedFile $file, &$errors) | ||
| { | ||
| // Explicitly reject archive files for security | ||
| if (config('files.enable_dangerous_validation')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Default Validation Setting Insecure
The enable_dangerous_validation check in validateFile lacks a default true value, unlike other validation configurations. This results in dangerous file validation, like rejecting archive files, being disabled by default, which creates a security vulnerability.
|
QA server K8S was successfully deployed https://ci-fb8ea55a17.engk8s.processmaker.net |
1 similar comment
|
QA server K8S was successfully deployed https://ci-fb8ea55a17.engk8s.processmaker.net |
|
|
@marcoAntonioNina please check the comments from cursor, and add the corresponding comments if those are not applicable |





Issue & Reproduction Steps
There not should be possible to upload dangerous files from WE
Solution
How to Test
Test
Related Tickets & Packages
Code Review Checklist
ci:deploy
ci:package-webentry:bugfix/FOUR-26797
ci:package-collections:bugfix/FOUR-26797