Primarly rely on magic to determine mime type

[MarcelEeken]

I have been investigating some issue lately about file uploads and mimetype determination and was looking at how CarrierWave implements it, and also how other projects implement it.

I saw in the source code of CarrierWave that it primarly relies on the given content_type of the file being attached

carrierwave/lib/carrierwave/sanitized_file.rb

Lines 261 to 266 in d50d80e

	def content_type
	@content_type ||=
	existing_content_type ||
	marcel_magic_content_type ||
	mini_mime_content_type
	end

.

As most files used by CarrierWave are potentially uploaded this can be an entry point for abuse, as the given content_type can be easily spoofed, as also has been mentioned here #1942 (comment).

With the recognition here #2465 (comment) as well that spoofing protection will become more important is it maybe an idea to not rely on the existing content type, and fully rely on the content type given by tools such as Marcel?

This is by the way how it is now solved in ActiveStorage https://github.com/rails/rails/blob/e848e8861d5c9221b77e039c2f041abced3aa577/activestorage/app/models/active_storage/blob/identifiable.rb#L21-L23

[aquintino]

Is there any official patch/solution to this issue? I am thinking of patching sanitized file to ignore the existing content type if file is an instance of ActionDispatch::Http::UploadedFile

[varentsov]

any updates on that issue?

[mshibuya]

Quite agree, changed in a2ca59c 👍