You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Situation
ActiveStorage uses Marcel#for to determine the mime type of an uploaded file. Marcel#for tries to determine the mime type from the given io object or path (in the case of ActiveStorage an io object is always supplied). If that fails (because Marcel cannot determine the type using its magic headers), it falls back to using the file's extension or content type - in case of ActiveStorage both of these are user supplied.
So as long as an uploaded file doesn't match any of the magic headers, the user is free to supply any mime type he chooses, posing a potential way to upload malicious files and disguising them as different mime types (also concerns mime type validations)
The else-block always falls back to the mime times derived from user supplied information about the file, even if an io object or path was supplied.
What do you think about this? Am I missing something?
Fix
From my current point of view, I'd suggest returning the BINARY (constant) type if no mime type could be extracted from a supplied io object or path.
...
else# when an io object or pathname were supplied but type could not be determined from it,# don't use any fallback type extracted from other supplied information.pathname_or_io ? BINARY : fallback_typeend
The text was updated successfully, but these errors were encountered:
gr8bit
changed the title
Potential minor security risk (upload arbitrary data) with ActiveStorage
Potential minor security risk (upload/spread arbitrary data) with ActiveStorage
Sep 10, 2022
Situation
ActiveStorage uses
Marcel#for
to determine the mime type of an uploaded file.Marcel#for
tries to determine the mime type from the given io object or path (in the case of ActiveStorage an io object is always supplied). If that fails (because Marcel cannot determine the type using its magic headers), it falls back to using the file's extension or content type - in case of ActiveStorage both of these are user supplied.So as long as an uploaded file doesn't match any of the magic headers, the user is free to supply any mime type he chooses, posing a potential way to upload malicious files and disguising them as different mime types (also concerns mime type validations)
This is the method in question:
marcel/lib/marcel/mime_type.rb
Lines 29 to 38 in 8e28563
The
else
-block always falls back to the mime times derived from user supplied information about the file, even if an io object or path was supplied.What do you think about this? Am I missing something?
Fix
From my current point of view, I'd suggest returning the BINARY (constant) type if no mime type could be extracted from a supplied io object or path.
The text was updated successfully, but these errors were encountered: