Ensure image MIME type matches extension

[Viper007Bond]

Take a bitmap (BMP) and rename it to .png. WordPress will say it's an image/png everywhere you look. This can cause issues if you're trying to manipulate it (thumbnail it, etc.).

We should either fix the extension or reject it.

[Viper007Bond]

Potential solution

[Viper007Bond]

Above patch fixes the extension. Tested and seems to work great.

[Viper007Bond]

This patch requires GD and should therefore be wrapped in if ( function_exists('getimagesize') ) {. The patch is just a preliminary one though, so I won't bother updating it yet.

[ShaneF]

I took a "png" renamed it to "gif" (with patch). Uploaded. File was changed to the correct format, but in the editor it still says "File type: image/gif". Perhaps override the file type as well?

[ShaneF]

This is a potential security problem. Recommending 2.9.2 release fix.

[Viper007Bond]

Patch improvements, likely needs porting to wp_handle_sideload() too

[Viper007Bond]

Replying to ShaneF:

This is a potential security problem. Recommending 2.9.2 release fix.

I can't think of a way to validate other extensions though, i.e. say a EXE uploaded as an AVI.

[hakre]

Don't we have an array already in there that is offering mime-types per extension? I think it's adviseable to use the same datasource here therein the patch as well instead offering duplicate data.

[Viper007Bond]

Replying to hakre:

Don't we have an array already in there that is offering mime-types per extension? I think it's adviseable to use the same datasource here therein the patch as well instead offering duplicate data.

That array (in get_allowed_mime_types()) is a list of allowed MIMEs that's filterable. It also has multiple extensions to one common MIME. Yes, I guess I could array_flip() it and then pop off the first extension, however I didn't think this was reliable enough due to the filter.

[Viper007Bond]

Replying to Viper007Bond:

That array (in get_allowed_mime_types()) is a list of allowed MIMEs that's filterable. It also has multiple extensions to one common MIME. Yes, I guess I could array_flip() it and then pop off the first extension, however I didn't think this was reliable enough due to the filter.

Actually, I take that back, it could work now that I think of it.

[hakre]

Well I did not say that it is fitting per-se, but worth to take a look into it. I mean why duplicate data here? But no idea if it is available in the location you would like to use it. We might need to introduce another filter here (or isn't there already one?!). Might make sense in the one or other way.

[Viper007Bond]

Now I remember why I just made a new array -- it's the list of image types that the function can identify. No point in using the other array.

[Viper007Bond]

Minor code improvements

[nacin]

(In [14400]) Ensure image MIME type matches extension for images. props Viper007Bond, fixes #11946.

[Viper007Bond]

Should be moved to a helper function and added to the other upload functions (sideload and the bits one).

[westi]

Bumping the need for a patch up the importance radar

[Viper007Bond]

Introduce new helper function and use it in all 3 upload functions

[Viper007Bond]

11946.4.patch introduces wp_check_filetype_and_ext() which does what wp_check_filetype() does, but then goes on to attempt to validate the type and extension. If it determines that the extension doesn't match what the type truly is, then it'll return proper_filename as a part of it's returned array.

I've modified the standard upload function, the sideload function, and the bits function to make use of this new function.

I have only tested this using the standard upload process, not sideload or bits.

[nacin]

(In [14649]) Introduce wp_check_filetype_and_ext() to handle mime/ext image comparisons and corrections for upload and sideload. props Viper007Bond, see #11946.

[nacin]

Viper007Bond and I have thoroughly tested this for sideloads and uploads. Weary about bits, so going to handle that no sooner than 3.1.

[hakre]

3.1 now on shedule. maybe optionally support ​fileinfo?

[nacin]

Just need to get the existing patch working for bits. Probably would be good for Joseph to take a look.

[hakre]

Patch is broken.

[ryan]

[14649]

[nacin]

Last bit is needed.

[markoheijnen]

Added a fix for wp_upload_bits at ticket #21292

[jackreichert]

If extension does not match mime type, rejects upload.

[jackreichert]

I noticed that you can upload a file with the wrong extension. The function wp_check_filetype_and_ext() in wp-includes/functions.php says that it does this, but it does not. I added a few lines in the above patch that fixes this security bug.

Note, it relies on finfo_file. To make sure that it won't break servers running php < 5.3 I wrapped the code in function_exists().

[Viper007Bond]

It's better to rename than reject the upload, especially since that's what the other upload functions already do. We should also be as helpful to the user as possible (they're trying to upload a file, why not let them?). My original patch does this: attachment:11946.4.patch​

Additionally as @markoheijnen mentions above, #21292 has a patch on it which addresses this ticket. It's based on my patch but uses extract() (boo!) but ends up accomplishing the same thing. However it also makes some more sweeping changes and there doesn't seem to be much traction on that ticket.

Perhaps it'd be better to just implement the changes as originally suggested in attachment:11946.4.patch​ and loop back to #21292 at a later point.

[markoheijnen]

Sorry about the extract() part ;). Making decisions based on traction isn't a good way. If we address it then everything should be addressed. We could close that ticket and merge it with this code.

[MikeHansenMe]

see #30284

[joemcgill]

In 39831:

Media: Improve image filetype checking.

This adds a new function wp_get_image_mime() which is used by
wp_check_filetype_and_ext() to validate image files using
exif_imagetype() if available instead of getimagesize().

getimagesize() is less performant than exif_imagetype() and is
dependent on GD. If exif_imagetype() is not available, it falls back to
getimagesize() as before.

If wp_check_filetype_and_ext() can't validate the filetype, we now return
false for ext/MIME values.

See #11946.

[gitlost]

You've got ! is_callable( 'exif_imagetype' ) !

[joemcgill]

Good catch @gitlost. Leftover from debugging. 11946-iscallable.diff​ fixes this issue.

[joemcgill]

In 39850:

Media: Fix exif_imagetype check in wp_get_image_mime

This is a follow up to [39831].

Props gitlost.
See #11946.

[audrasjb]

As per today's old tickets triage session:

It appears this ticket was fixed by the above commit. Closing as fixed.
Follow-up changes are being handled in #21292.

Thanks everyone!