Feature #43484
closed
Detect attachment content type from file contents instead of trusting client-provided values
Added by Go MAEDA 10 months ago. Updated 18 days ago.
Description
Redmine currently stores the content type of uploaded files based on the value provided by the client, such as web browsers and API clients. This approach is not reliable and can lead to incorrect or misleading MIME types, especially when the client declares a spoofed or incorrect type.
According to the OWASP File Upload Cheat Sheet, the Content-Type header for uploaded files cannot be trusted, because it is easy to spoof.
The attached patch changes the behavior so that Redmine no longer trusts the client-provided content type. Instead, it detects the content type by inspecting the actual file contents using Marcel.
Files
Related issues
Updated by Go MAEDA 10 months ago
- Related to Feature #43473: Reject file uploads when actual MIME type does not match the file extension added
Updated by Go MAEDA 6 months ago
- File 0001-Improve-Redmine-MockFile-to-better-mimic-File-behavi.patch 0001-Improve-Redmine-MockFile-to-better-mimic-File-behavi.patch added
- File 0002-Stop-using-legacy-image-x-ms-bmp-MIME-type-for-BMP-f.patch 0002-Stop-using-legacy-image-x-ms-bmp-MIME-type-for-BMP-f.patch added
- File 0003-Stop-using-legacy-application-x-pkcs7-signature-MIME.patch 0003-Stop-using-legacy-application-x-pkcs7-signature-MIME.patch added
- File 0004-Detect-attachment-content-type-from-file-contents-in.patch 0004-Detect-attachment-content-type-from-file-contents-in.patch added
- File deleted (
0001-Improve-Redmine-MockFile-to-better-mimic-File-behavi.patch) - Target version set to Candidate for next major release
- File deleted (
0002-Stop-using-legacy-image-x-ms-bmp-MIME-type-for-BMP-f.patch) - File deleted (
0003-Stop-using-legacy-application-x-pkcs7-signature-MIME.patch) - File deleted (
0004-Detect-attachment-content-type-from-file-contents-in.patch)
I have updated the patches. Rebased for the current trunk and made minor changes.
Updated by Marius BĂLTEANU 2 months ago
- Target version changed from 7.0.0 to 7.1.0
Updated by Go MAEDA about 1 month ago
- File 0001-Improve-Redmine-MockFile-to-better-mimic-File-behavi.patch 0001-Improve-Redmine-MockFile-to-better-mimic-File-behavi.patch added
- File 0002-Stop-using-legacy-image-x-ms-bmp-MIME-type-for-BMP-f.patch 0002-Stop-using-legacy-image-x-ms-bmp-MIME-type-for-BMP-f.patch added
- File 0003-Stop-using-legacy-application-x-pkcs7-signature-MIME.patch 0003-Stop-using-legacy-application-x-pkcs7-signature-MIME.patch added
- File 0004-Show-the-same-file-type-icon-whether-the-content-typ.patch 0004-Show-the-same-file-type-icon-whether-the-content-typ.patch added
- File 0005-Detect-attachment-content-type-from-file-contents-in.patch 0005-Detect-attachment-content-type-from-file-contents-in.patch added
- File 0006-Send-an-attachment-inline-based-on-its-detected-cont.patch 0006-Send-an-attachment-inline-based-on-its-detected-cont.patch added
Updated the patches.
Two problems became visible once the content type is detected from the file contents, and each is fixed in its own commit.
Attachment icons:
Marcel does not always spell a type the way Redmine::MimeType spells the type of the same kind of file. For example, Marcel returns text/javascript where Redmine::MimeType returns application/javascript. The icon name was derived from the content type, so files like this lost their icon and fell back to the generic one. The icon is now looked up in a table that lists every spelling.
Inline display:
Content-Disposition was decided from the file extension while Content-Type was decided from the detected content type, so the two could disagree. An HTML file named foo.pdf was sent as text/html with Content-Disposition: inline. Both are now decided from the same value.
Other fixes:
- The content type is re-detected on every rename path. Previously only attaching by token did this, so renaming from the attachment edit screen or through the REST API left a stale content type.
- Renaming no longer raises an error when the file is missing from disk.
Updated by Go MAEDA 18 days ago
- Status changed from New to Resolved
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patches in r24955, r24956, r24957, r24958, r24959, and r24960.
Redmine no longer stores the content type sent by the client. Instead, Redmine now reads the file itself and detects its type with Marcel. This applies to all upload paths: the web interface, the REST API, and incoming email.
This improves security because the content type reported by a client cannot be trusted and can easily be spoofed. Redmine now determines the content type from the file itself instead of relying on information supplied by the client. See Content-Type Validation in the OWASP File Upload Cheat Sheet.
Please note that the content type sent by REST API clients when uploading attachments is now silently ignored. Instead, Redmine detects the content type from the uploaded file. Please note that the content type sent by REST API clients when uploading attachments is now silently ignored. Instead, Redmine detects the content type from the uploaded file. No error is returned when a content type is specified, so existing clients do not need to change their requests.
Updated by Go MAEDA 9 days ago
- Related to Feature #44385: Replace MiniMime with Marcel in Redmine::MimeType for consistent MIME type lookup added