Check that uploaded file content matches the file extension
Problem to solve
In the past, we have had several problems when the uploaded file content doesn't match the file extension, ie different Content-Type and Content-Disposition headers.
We should check the integrity of the file before storing the file and reject the upload.
Proposal
A quick fix (I haven't tested edge cases or whether there is a better approach) would be:
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 7dc211b14e4..aaf4e6ec7cf 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -3,6 +3,8 @@
class GitlabUploader < CarrierWave::Uploader::Base
class_attribute :options
+ before :cache, :check_content_type_integrity!
+
class << self
# DSL setter
def storage_options(options)
@@ -136,4 +138,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
def pathname
@pathname ||= Pathname.new(path)
end
+
+ def check_content_type_integrity!(file)
+ if MimeMagic.by_magic(file.read) != MimeMagic.by_path(filename).type
+ raise CarrierWave::IntegrityError, 'Content type does not match file extension'
+ end
+ end
end
Nevertheless, we should change the FE part to handle exceptions coming from the uploader. Otherwise, the error will be displayed:
We could also move this process to Workhorse and avoid any Rails work completely but we have had problems detecting content types in Workhorse before and the Rails libraries are more complete. However, we should consider this option.
Edited by Francisco Javier López
