Make WordPress Core

Opened 7 years ago

Last modified 5 months ago

#40175 assigned defect (bug)

Upload Validation / MIME Handling

Reported by: blobfolio's profile blobfolio Owned by:
Milestone: Future Release Priority: high
Severity: major Version: 4.7.3
Component: Media Keywords: has-unit-tests early needs-dev-note
Focuses: administration Cc:

Description

A security fix implemented in WordPress 4.7.1 relies on a PHP extension (fileinfo) with inconsistent reporting behavior. As a result, many users (even after #39550) trying to upload various types of files (office documents, multimedia, fonts, etc.) have received validation errors.

In a nutshell, this is because the media types returned by fileinfo vary from server to server and file to file. If PHP returns a media type beginning application/*, that media type must be whitelisted or the result will fail.

Because most incorrect/historical answers from fileinfo begin application/*, this is resulting in a large number of false-positives.

There are three main ways to address this, with a combination approach being preferred:

1) The conditional in wp_check_filetype_and_ext could be restricted so that rather than searching application/* broadly, it looks only at the narrow file types at the heart of the original security issue. This option requires review from the Security Team.

2) The WordPress Core could be extended to provide "MIME alias" awareness. This would allow WordPress to properly match a given extension/MIME pairing even in cases where the MIME type is historically valid, but not the singular type in the whitelist. See #39963 for related information.

3) WP could be extended to maintain its own mime.types file, which can be passed to fileinfo, providing more consistent responses. This option requires the MIME alias handling to avoid breaking sites or plugins which hook into upload_mimes.

Duplicate/related tickets are being collapsed into this thread. Please continue all related discussion here.

Attachments (7)

002713876-0.XLS (9.0 KB) - added by lovelucy 7 years ago.
cannot upload xls with strange MIME: application/CDFV2-unknown
40175.diff (11.3 KB) - added by blobfolio 7 years ago.
Implements an explicit "greylist" for file types requiring additional validation, rather than scrutinizing every application/* type.
40175.2.diff (11.3 KB) - added by blobfolio 6 years ago.
Rebuilt patch against current src
phpunit-file.zip (903 bytes) - added by blobfolio 6 years ago.
Unit test file (in case --binary from the .diff causes trouble)
40175.3.diff (42.9 KB) - added by joemcgill 5 years ago.
40175.4.diff (45.5 KB) - added by joemcgill 5 years ago.
40175.5.diff (47.9 KB) - added by joemcgill 5 years ago.

Download all attachments as: .zip

Change History (95)

#1 @blobfolio
7 years ago

#40078 was marked as a duplicate.

#2 @blobfolio
7 years ago

#40170 was marked as a duplicate.

#3 @blobfolio
7 years ago

#40101 was marked as a duplicate.

#4 @blobfolio
7 years ago

  • Severity changed from normal to critical

#5 follow-up: @joemcgill
7 years ago

  • Owner set to joemcgill
  • Status changed from new to accepted

Thanks for consolidating all of this here @blobfolio.

It's probably helpful to define some base assumptions here. Before [39831], WordPress essentially trusted that all uploaded files were exactly what they claimed to be, based on the file extension. The only exception is that image files were verified in an attempt to rename image files that had accidentally been saved with the wrong extension (see #11946). From what I can tell, this was mainly a UX improvement when working with images, to avoid editor errors and was not strict about allowing uploads based on actual mime types.

#6 in reply to: ↑ 5 @blobfolio
7 years ago

Replying to joemcgill:

It's probably helpful to define some base assumptions here. Before [39831], WordPress essentially trusted that all uploaded files were exactly what they claimed to be, based on the file extension.

I agree.

Prior to 4.7.1, WordPress itself was the only source of information about a file's media type, so all file handling had a predictable framework to deal with. But now that an outside source of information has been added (two actually), we've crossed the Rubicon.

If any amount of outside information is to be used by WordPress, there needs to be a system in place to reconcile that information with WordPress' own whitelisting system. Otherwise any contradiction will result in failure.

That isn't addressed by (1), however limiting the amount of outside information being used for validation will result in fewer innocent files being mistakenly caught in the net.

(2) tackles the problem directly by acting as a sort of Babel Fish, and will help futureproof the platform. It also affords opportunities for later enhancements, such as more aggressive upload validation or (3) below.

(3) is more of an enhancement than an immediate fix to the problem. It would be helpful for data consistency, but would create conflicts with existing sites and plugins that have extended their whitelists (unless we have (2) already in place).

From what I can tell, this was mainly a UX improvement when working with images, to avoid editor errors and was not strict about allowing uploads based on actual mime types.

Definitely. (Correctly) renaming images before handing them off to the thumbnail components is necessary to avoid errors. Insofar as I can tell, this piece has worked as expected since 4.7.1.

The only image-related upload validation issues were due to unusual types like WEBP or SVG that individuals may have added to their whitelists. #39550 fixed that, except in cases where the server returns an (incorrect) application/* type for the file.

@lovelucy
7 years ago

cannot upload xls with strange MIME: application/CDFV2-unknown

#7 follow-up: @lovelucy
7 years ago

@blobfolio Related to #39550, I found another xls that cannot be uploaded to WordPress, which is tested with result MIME: application/CDFV2-unknown

File attached

#8 in reply to: ↑ 7 @blobfolio
7 years ago

Replying to lovelucy:

@blobfolio Related to #39550, I found another xls that cannot be uploaded to WordPress, which is tested with result MIME: application/CDFV2-unknown

Awesome, thanks @lovelucy! It seems CDFV2 is another hardcoded non-MIME MIME living inside the fileinfo.so extension, only unlike application/ms-office, it gets variable bits tacked onto the end like "unknown" and "corrupt". I'll add a workaround of some sort to the #39963 patch.

#9 follow-up: @hivewebstudios
7 years ago

For what it is worth, our plugin https://wordpress.org/plugins/font-organizer/ is suffering heavily from this bug, and a lot of our users are reporting issues with uploading fonts to their WP.
Right now our official instruction is to actually download the plugin https://wordpress.org/plugins/disable-real-mime-check/ to sort that out, but obviously it is not an actual solution.
Is there some easy way around for us to implement a solution within the plugin that will allow our users to upload fonts without getting that error?

We first figured we would just wait for 4.7.3 to solve that issue but since its ongoing, we need some other elegant solution that won't affect users later on when thte bug is solved and also will minimize compatibility issues.

Also I truly hope I am not breaking any rules by posting this, I never had to deal with core issues before and we are currently testing our next major plugin release.

#10 in reply to: ↑ 9 @blobfolio
7 years ago

Replying to hivewebstudios:

For what it is worth, our plugin https://wordpress.org/plugins/font-organizer/ is suffering heavily from this bug, and a lot of our users are reporting issues with uploading fonts to their WP.

Yeah, unfortunately font files are particularly likely to be hit with this issue. Prior to IANA's designation of a font/ type, many fell under the application/ banner, and a lot of different versions of PHP will still see them that way.

In your case, at least, there is a possible workaround. The upload validation code causing the trouble includes a filter; your plugin could hook into this and alter the response to "A-OK" for font files. Please see https://core.trac.wordpress.org/ticket/39550#comment:110, which worked for another author.

Also I truly hope I am not breaking any rules by posting this, I never had to deal with core issues before and we are currently testing our next major plugin release.

No worries! It is helpful when users share their own experiences.

#11 @blobfolio
7 years ago

The proposed fix (via #39963) has been repackaged as a plugin for broader testing! (The ticket also includes the changes as a patch if you'd rather apply it that way.)

For anybody affected by the issues in this ticket, please give it a try: https://core.trac.wordpress.org/attachment/ticket/39963/blob-mimes.zip

If you install it, please be sure to follow #39963 so you are kept abreast of plugin updates, if any. And of course, report any happy or sad news from your use there.

For reference, it is pretty much the opposite of the earlier quick-fix https://wordpress.org/plugins/disable-real-mime-check/. It does not disable the security features introduced in 4.7.1 — in fact it greatly expands them — but instead is much smarter about reconciling the variation in type detection that exists from server to server.

@lovelucy, this also addresses the detection inconsistencies we've seen within a single file "type". Please let me know if it fixes your Excel document upload issues.

Thanks everyone!

#12 @blobfolio
7 years ago

  • Focuses administration added

The plugin implementing the proposed fix (via #39963) can now be found in the main WordPress Plugins repository: https://wordpress.org/plugins/blob-mimes/

If you had already installed version 0.1.2 directly from the ticket, you should automatically see the usual orange upgrade notice within the next 24 hours, which will transition you to the WP-hosted version.

If you are currently using an older version or running it from a server that can't reach Github (the previous host), please uninstall the plugin and reinstall it from the official WordPress Plugins repo to catch up.

The new release includes a handy upload debug tool for users with manage_options privileges (e.g. admins) under Tools > Debug File Validation. If at any point in the future the Media Library is rejecting something it shouldn't, please use that tool to gather some useful feedback information and post it to #39963 (or email me if you aren't comfortable posting it publicly).

Thanks!

Last edited 7 years ago by blobfolio (previous) (diff)

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


7 years ago

#14 in reply to: ↑ 13 ; follow-up: @wjlonien
7 years ago

Replying to slackbot:

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.

Works like a charm - now I can finally upload .ogg files to WP4.8. Thanks @joemcgill and @blobfolio !

#15 in reply to: ↑ 14 @blobfolio
7 years ago

Replying to wjlonien:

Works like a charm - now I can finally upload .ogg files to WP4.8. Thanks @joemcgill and @blobfolio !

Woo! Yeah, ogg is a tough format, particularly the ogx variant. They can really confuse the heck out of servers (which is odd, you'd think with Linux's domination in that area ogg would get first-class treatment!). Haha.

If you ever stumble across a random ogg file that doesn't work, or any other kind of file, please let me know! Lord of the Files comes with a handy admin tool for that: Tools > Debug File Validation. The information spit out by that page can be emailed to me or posted to https://github.com/Blobfolio/blob-mimes, whichever is easier for you. Thanks!

@blobfolio
7 years ago

Implements an explicit "greylist" for file types requiring additional validation, rather than scrutinizing every application/* type.

#16 @blobfolio
7 years ago

  • Keywords has-unit-tests has-patch needs-testing added

This is a long thread, so here's a quick summary:

In response to a security issue, WordPress 4.7.1 added some content-based evaluations for uploaded media. That implementation affects any file that PHP says is application/*. Because PHP is not very good or consistent at this type of detection, a lot of legitimate files are also being blocked.

This patch instead switches to a "greylist" system, containing explicit extensions and MIMEs (any extension can have multiple types) deserving further scrutiny. When a file is uploaded, the content-derived type is compared against the greylist, and if it matches, only then will it dive deeper and maybe block the upload.

This approach both limits collateral damage and helps WP find more of the sorts of files it was looking for in the first place.

What we need now are some eyeballs!

Please and thank you!

PS: Just be sure to temporarily disable Lord of the Files if your test site has that installed. (And don't forget to reactivate it afterwards; it contains a number of other file-related security improvements unrelated to this ticket. Haha.)

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


7 years ago

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


7 years ago

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


7 years ago

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


7 years ago

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


7 years ago

#22 @joemcgill
7 years ago

  • Milestone changed from Awaiting Review to 4.9

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


6 years ago

@blobfolio
6 years ago

Rebuilt patch against current src

@blobfolio
6 years ago

Unit test file (in case --binary from the .diff causes trouble)

#24 @blobfolio
6 years ago

It has been a while since the initial patch was put together, so it has been regenerated to make it easier to apply against the current WordPress source.

40175.2.diff includes binary data (a small file for the unit tests). That data can be ignored if it causes problems with your version control software; the relevant file is also provided in the phpunit-file.zip attached to this ticket.

Just a reminder, if testing this patch out, be sure to temporarily disable the Lord of the Files plugin if it is part of the test site. (The plugin independently fixes this and other issues, so there'd be no way to know whether the patch worked if it were active. Haha.)

Thanks!

This ticket was mentioned in Slack in #core-media by blobfolio. View the logs.


6 years ago

#26 @joemcgill
6 years ago

  • Keywords early added
  • Milestone changed from 4.9 to 4.9.1
  • Severity changed from critical to major

#27 @joemcgill
6 years ago

Punting this to the next minor to ensure we get some more soak time. My fault on letting this go so long.

#28 @blobfolio
6 years ago

It seems like a more complete solution to the various MIME-related challenges is back on the table. For those interested, keep an eye on #39963, which if accepted, would moot the patch that was uploaded to this ticket.

#29 @johnbillion
6 years ago

  • Milestone changed from 4.9.1 to 5.0

#30 @okvee
6 years ago

I come to this bug from using wp_handle_upload() function.

My code:

$overrides['action'] = $_POST['action'];
$overrides['mimes'] = [
    'jpg|jpeg|jpe' => 'image/jpeg',
    'gif' => 'image/gif',
    'png' => 'image/png',
];
$uploadResult = wp_handle_upload($_FILES['meter_elec_photo_url'], $overrides);

I have fake .jpg image file which really is .txt text file, I try to upload using this code and it allow me to upload without checking the real file's mime type.

I follow into wp_handle_upload() and _wp_handle_upload() functions. It use wp_check_filetype_and_ext() function to check file extension.

Other than this if condition:

0 === strpos( $real_mime, 'application' )

I think instead of currently using $allowed = get_allowed_mime_types();, it is better use $mimes.

Example:

if (empty($mimes)) {
   $allowed = get_allowed_mime_types();
} else {
    $allowed = $mimes;
}

With this code, this function can use inherit $mimes value from wp_handle_upload() function perfectly and safety.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 years ago

#32 @peterwilsoncc
5 years ago

  • Milestone changed from 5.0 to 5.1

Moving to the 5.1 milestone due to the WordPress 5.0 focus on the new
editor (Gutenberg).

#33 @greatislander
5 years ago

I haven't run into this issue in a really long time but we just got a report of a valid MP3 failing validation from a user last week. I see this is in 5.1 now—anything I can do to help with testing and moving this along?

#35 follow-up: @KZeni
5 years ago

I can also report that I had a small & very typical CSV unable to be uploaded to a site's Media Library running the current WordPress version (5.0.2) with "Sorry, this file type is not permitted for security reasons." shown as the reason. I even have a plugin which specifically has CSV files set to be allowed for upload.

Implementing https://wordpress.org/plugins/blob-mimes/ resolved the issue for now (thanks @blobfolio !), but I'd imagine this should be addressed in the WordPress core.

#36 in reply to: ↑ 35 ; follow-up: @blobfolio
5 years ago

Replying to KZeni:

Implementing https://wordpress.org/plugins/blob-mimes/ resolved the issue for now (thanks @blobfolio !), but I'd imagine this should be addressed in the WordPress core.

I agree, but unfortunately this just isn't a Core priority, at least not for 2019 https://make.wordpress.org/core/2018/12/08/9-priorities-for-2019/.

I'll be maintaining Lord of the Files indefinitely, so that'll at least remain an option. :)

#37 in reply to: ↑ 36 @iandunn
5 years ago

  • Priority changed from normal to high

FWIW, the CSV issue in comment:35 is being worked on in #45615, and looks like it's on track to be fixed in an upcoming maintenance release.

#38 @iandunn
5 years ago

  • Priority changed from high to normal

Doh, I didn't mean to alter the priority in my previous comment. Reverting that.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 years ago

@joemcgill
5 years ago

#40 @joemcgill
5 years ago

40175.3.diff is a first pass at a new approach to this problem, heavily influenced by all of @blobfolio's previous work on this issue. The overall idea is to simplify/consolidate the logic in wp_check_filetype_and_ext() so that there are distinct separation of concerns between managing the mime types known to WordPress, guessing the mime type of a file, possibly renaming a file that has the wrong extension, and determining if a file should be allowed.

Managing registered mime types: This would happen in wp_get_file_types(), which is similar to the old wp_get_mime_types(), but supports an array of supported mime types for each file extension.

Guessing a file's mime type: This was previously handled internally in wp_check_filetype_and_ext() with some logic previously being consolidated to wp_get_image_mime(). This would create a new function called wp_get_mime_type() which loops through a list of registered "mime type guessers" to determine the actual mime type. One of the guessers is, naturally, wp_get_image_mime(). Additionally, all other finfo_file() logic is consolidated to a new function, wp_get_file_mime().

Possibly rename files with the wrong extension: Traditionally, wp_check_filetype_and_ext() has attempted to fix some image files if they are named incorrectly. This functionality would now be consolidated to wp_maybe_rename_file(), though, I'm curious if we really need to maintain this behavior anymore.

Determining if a file type is allowed: This now becomes a simple check in wp_is_file_type_allowed() by making use of the new wp_get_allowed_file_types() function, which grabs the full list of known mime types and filters out any types that aren't supported based on extensions or user capabilities.

Most of these changes are cosmetic, simplifying the code and making it easier to test. That said, there are two major changes worth noting if we go this route.

  1. WordPress can now map a single extension to multiple supported mime types.
  2. If a file cannot be verified or if the mime type is not supported for that extension, the file will not be able to be uploaded.

Plugins will be able to extend both the file type list, and the means of guessing file types, in order to modify default behavior and I've attempted to support most cases where plugins were already doing the former by way of the mime_types and upload_mimes filters.

Next steps

I would appreciate feedback from people who are currently having problems uploading files to see if this approach fixes many of the blocked uploads problems we're having. If this approach works, we'll need to determine the best approach for deprecating the previous mime type functions in core (currently there are a few places I assume we will want to deprecate, but those are commented out).

#41 follow-up: @blobfolio
5 years ago

Thanks @joemcgill!

I had stopped updating the patches for this ticket while all eyes were on Gutenberg so the aliases were probably a bit out of date when you did your refresh.

A complete (and regularly updated list) can be taken from https://plugins.svn.wordpress.org/blob-mimes/trunk/lib/blobfolio/wp/bm/mime/aliases.php if needed.

#42 in reply to: ↑ 41 @joemcgill
5 years ago

Replying to blobfolio:

Oh, I already poached this list of aliases from your plugin, rather than previous patches ;)

Thanks for the heads up though!

#43 follow-up: @okvee
5 years ago

[Out of topic]How do I stop receive email from WordPress trac such as this ticket?

Blocked ticket, unwatched but still receive email for every reply.

#44 in reply to: ↑ 43 ; follow-up: @joemcgill
5 years ago

Replying to okvee:

[Out of topic]How do I stop receive email from WordPress trac such as this ticket?

Blocked ticket, unwatched but still receive email for every reply.

I'd suggest looking at your settings on https://make.wordpress.org/core/notifications/ to ensure there's nothing else causing you to get notifications for this ticket. If not, there might be a Trac bug needing to be fixed.

#45 in reply to: ↑ 44 @okvee
5 years ago

Replying to joemcgill:

Replying to okvee:

[Out of topic]How do I stop receive email from WordPress trac such as this ticket?

Blocked ticket, unwatched but still receive email for every reply.

I'd suggest looking at your settings on https://make.wordpress.org/core/notifications/ to ensure there's nothing else causing you to get notifications for this ticket. If not, there might be a Trac bug needing to be fixed.

Yes, I'm already look at that too. Nothing were checked.
This page https://core.trac.wordpress.org/report/51 and this page https://core.trac.wordpress.org/report/50 contain nothing.
I'm still getting email notify here.

If this is Trac bug and how do I stop receive email or what I have to do?

#46 follow-up: @harmr
5 years ago

@joemcgill thanks for the update! I tried your patch to verify if this fixes our GPX file upload issue, unfortunately no change - file type .gpx still not permitted due to security reasons. We haven´t changed how we register the GPX mime types, details can be found here:
https://core.trac.wordpress.org/ticket/45615#comment:45

#47 in reply to: ↑ 46 @joemcgill
5 years ago

Replying to harmr:

@joemcgill thanks for the update! I tried your patch to verify if this fixes our GPX file upload issue, unfortunately no change - file type .gpx still not permitted due to security reasons. We haven´t changed how we register the GPX mime types, details can be found here:
https://core.trac.wordpress.org/ticket/45615#comment:45

Thanks @harmr. In the case of this approach—and also with the current code in WP—registering your extension (gpx) with a supported MIME type (application/gpx+xml) is not enough. The server also has to verify that the file actually is the MIME type that is supported for that extension. My hunch is that you would need register the extension with a MIME type of application/xml instead of applciation/gpx+xml, but I would have to test that to be sure. With this new system, you could do something like this to support both possibilities:

add_filter( 'wp_mimes', function( $mimes ) {
  $mimes['gpx'] => array(
    'application/gpx+xml',
    'application/xml',
  );

  return $mimes;
} );

Note that the current proposed filter name is likely to change before this is finalized.

Last edited 5 years ago by joemcgill (previous) (diff)

#48 @joemcgill
5 years ago

  • Keywords needs-dev-note added

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


5 years ago

#50 follow-up: @pento
5 years ago

Overall, I like the direction of this solution. 🙂

I think we need to keep the functionality in wp_maybe_rename_file(), but it probably needs a more descriptive name. 😉

Is there a particular reason why the array in wp_get_file_types() isn't keyed the same way as the array in wp_get_mime_types()? This would remove a bunch of duplicate entries. Alternatively, perhaps something like this structure:

$mime_map = array(
        'jpg'     => array(
                'mime_types' => array(
                        'image/jpeg',
                        'image/pjpeg',
                ),
                'extensions' => array( 'jpg', 'jpeg', 'jpe' ),
        ),
        // ...
);

The wp_get_mime_types() array could be generated from wp_get_file_types(). Even if it's deprecated, we'd want to keep it updated for a while.

Logistically, I don't think this can be ready for 5.1: it's going to take a bit of work to integrate it into Core, and I suspect we're going to run into all sorts of interesting edge cases when plugins work to convert to it. I think it could easily be committed early to 5.2, though.

#51 in reply to: ↑ 50 @joemcgill
5 years ago

@pento – Thanks for the review.

I think we need to keep the functionality in wp_maybe_rename_file(), but it probably needs a more descriptive name. 😉

Agreed. I'll bike-shed it a bit.

Is there a particular reason why the array in wp_get_file_types() isn't keyed the same way as the array in wp_get_mime_types()? This would remove a bunch of duplicate entries. Alternatively, perhaps something like this structure:

The regex style keying has proven to be less robust than this solution should be, and adds needless complexity when trying to do key => value lookups in the array. Additionally, there are not often cases where the full array of possible MIME types will be identical, so in this case I've chosen a little extra verbosity for much more clarity and flexibility.

The wp_get_mime_types() array could be generated from wp_get_file_types(). Even if it's deprecated, we'd want to keep it updated for a while.

I agree that this is possible, but we'd need to decide and document which type to return as the default for each extension. Perhaps it's the first item in the array of MIMEs? Either way, we probably need to keep a map of default extensions to mime types for cases where validation isn't necessary.

Logistically, I don't think this can be ready for 5.1: it's going to take a bit of work to integrate it into Core, and I suspect we're going to run into all sorts of interesting edge cases when plugins work to convert to it. I think it could easily be committed early to 5.2, though.

This could be true. I'm still going to push for a committable patch by 5.1-beta-2 and we can make a decision in the next few days. Either way, it will put this in a better position for 5.2-early.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


5 years ago

@joemcgill
5 years ago

#53 @joemcgill
5 years ago

40175.4.diff is an iteration on 40175.3.diff which includes the following changes:

  • Removes application/octet-stream from the list of supported mime types for exe files.
  • Rename the filter in wp_get_file_types() from wp_mimes to wp_file_types.
  • Rename wp_maybe_rename_file to wp_maybe_fix_image_extension().
  • Fixes some bugs when filtering the list of mime types due to the early wp_check_filetype() check or improper extension renaming.
  • Adds a unit test for GPX files, which also serves as a test for the wp_file_types filter.

TODO:

  • Implement a deprecation strategy for wp_get_mime_types().
  • Decide how to handle unit tests that fail based on underlying server architecture.

@joemcgill
5 years ago

#54 @desrosj
5 years ago

Crossposting this for visibility. A comment on the 5.0.1 Backwards Compatibility Breaks post has raised issues with `.stp` and `.step` files.

#55 @joemcgill
5 years ago

40175.5.diff Adds backwards compatibility for the third $mimes parameter, which core never uses, but is sometimes used by plugins to pass in their own ext => mime type maps.

I think we should consider at least a soft deprecation of this parameter, since it bypasses any real mime checks, and instruct integrators to filter the list of mime guessers instead. Looking through the plugins directory, I don't see many uses of that third parameter that would break if we deprecated it.

#56 @dingo_bastard
5 years ago

I got this issue when trying to upload .yaml files that were 'downloaded' from a pulled github repo. The file type of the downloaded file is regarded as application/octet-stream.

Hope this will be solved in 5.1.

#57 @joemcgill
5 years ago

  • Milestone changed from 5.1 to 5.2

This is getting close, but not quite ready for 5.1, unfortunately. Moving to the 5.2 milestone.

#58 @earnjam
5 years ago

#46163 was marked as a duplicate.

#59 @julienmarchand
5 years ago

I am getting the same error with json file (and it's geojson cousin) and I see in the above diff that this format is not included... Thanks for the attention!

This ticket was mentioned in Slack in #core by desrosj. View the logs.


5 years ago

#61 @desrosj
5 years ago

#46282 was marked as a duplicate.

#62 @hphoeksma
5 years ago

See https://wordpress.org/support/topic/some-mime-types-not-working

.dox and .potx are changing mime_type in the proces (from .template to .document/.presentation) causing a mismatch between mime_type and extension.

This ticket was mentioned in Slack in #core-media by lukecarbis. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 years ago

#65 @jorbin
5 years ago

@joemcgill is this still something you are going to try and get in for 5.2? It's tagged early so it should likely be bumped, but you make the call.

#66 @joemcgill
5 years ago

  • Keywords has-patch needs-testing removed
  • Milestone changed from 5.2 to 5.3

@jorbin I wasn't able to complete this in time early in the 5.2 cycle, but have time to look at it again for 5.3. Bumping.

#67 @joemcgill
5 years ago

#46903 was marked as a duplicate.

#68 @joemcgill
5 years ago

  • Priority changed from normal to high

#69 @andraganescu
5 years ago

#47448 was marked as a duplicate.

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.


5 years ago

#71 @joemcgill
5 years ago

  • Milestone changed from 5.3 to Future Release
  • Owner joemcgill deleted
  • Status changed from accepted to assigned

I'm moving this out of the milestone in order to keep focus on other 5.3 priorities. If someone wants to pick this up for 5.3, please feel free.

#72 @afercia
5 years ago

Related: #47752.

#73 @joemcgill
5 years ago

#47946 was marked as a duplicate.

#74 @jesin
5 years ago

Today I faced an issue while uploading a docm file because fileinfo returns application/vnd.openxmlformats-officedocument.wordprocessingml.document as the MIME type while WordPress returns application/vnd.ms-word.document.macroEnabled.12.

I had to use the following workaround:

add_filter( 'upload_mimes', function( $mime_types ) {
        $mime_types['docm'] = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document';
        return $mime_types;
}, 1, 1 );

#75 @PerS
4 years ago

Just had an issue with uploading a password protected powerpoint presentation. I had the mime type set to application/vnd.openxmlformats-officedocument.presentationml.presentation. Uploading been working, but now I got the error "Unexpected response from the server. The file may have been uploaded successfully ...".
After debugging we found that the problem is in wp_check_filetype_and_ext and that a password protected powerpoint file is identified as application/vnd.ms-powerpoint

You can test this by doing:

$finfo     = finfo_open( FILEINFO_MIME_TYPE );
echo finfo_file( $finfo, 'passwordprotected.pptx' );
//output: application/vnd.ms-powerpoint
echo finfo_file( $finfo, 'not-passwordprotected.pptx' );
//output: application/vnd.openxmlformats-officedocument.presentationml.presentation

So, when will "MIME alias" awareness be added ?

Last edited 4 years ago by PerS (previous) (diff)

This ticket was mentioned in Slack in #core by soderlind. View the logs.


4 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 years ago

#79 @PerS
4 years ago

My temporary fix is:

<?php
add_filter( 'wp_check_filetype_and_ext', function( $wp_check_filetype_and_ext, $file, $filename, $mimes, $real_mime ) {
        if ( ! empty( $wp_check_filetype_and_ext['ext'] ) ) {
                return $wp_check_filetype_and_ext;
        }

        $allowed_mime_types = get_allowed_mime_types();
        /**
         * HACK: Replace mime type 'application/CDFV2[-*]' with the correct, if allowed, mime type.
         */
        if ( false !== strpos( $real_mime, 'application/CDFV2' ) ) {
                $ext = end( explode( '.', basename( $filename ) ) );
                if ( isset( $allowed_mime_types, $allowed_mime_types[ $ext ] ) ) {
                        $wp_check_filetype_and_ext = [
                                'ext'  => $ext,
                                'type' => $allowed_mime_types[ $ext ],
                        ];
                }
        }

        return $wp_check_filetype_and_ext;
}, 10, 5 );

#80 @clok
4 years ago

Wanted to add another file extension/type failing for me that I haven't seen reported yet:
test.css => text/css based on the array, but fileinfo is returning it as text/plain
WordPress 5.2.3
PHP 7.3.17 (Remi's)

Any CSS file will do. Adding text/css to the array within functions.php in the "elseif ( 'text/plain' === $real_mime )" code block will make it work again.

The troubling part about this bug (and its perpetual delay for a more perfect solution) for me is that while the logic changed to explicitly deny files whose extensions don't match their determined mime type, the interface for whitelisting didn't. So in vanilla multisite WordPress I am told I can state I want to allow "css" file extension uploads, but the code doesn't care because the file itself doesn't match behind the scenes and all I'm told is "not allowed for security reasons". If I had a field for telling it the mime types I want to allow matched with their extensions and/or the error was more explicit about what mime type my file was diagnosed as and thus why it was excluded, then I would have recourse without waiting for the solution to be implemented. I could add my own workarounds as needed for whatever obscure combination of mime type and file extension is holding up each of our work (and to be honest, uploading CSS files to WordPress isn't exactly obscure in my mind...).

EDIT: I've further discovered that when fileinfo thinks the .css file is assembly or c language (text/x-asm or text/x-c) instead of text/plain, then I have to introduce an entirely new elseif to handle those cases. This needs attention.

Last edited 4 years ago by clok (previous) (diff)

#81 @benz001
4 years ago

Not sure if this is best listed as a new feature request or a comment on this one, but to add another couple of examples to this: .DWG files can be 'dwg' => 'image/vnd.dwg' or 'dwg' => 'text/plain' similarly DXF files can be 'dxf' => 'text/plain' or 'dxf' => 'application/octet-stream' and possibly more. Both formats support text and binary versions of their extensions. SVGs can do a similar thing.

As far as I can tell the add_custom_mime_types filter allows for adding new extensions and for multiple extensions to map to the same type, but not the reverse ie multiple types per extension can't be added.

Hence the need discussed above (and here https://wordpress.stackexchange.com/questions/323750/how-to-assign-multiple-file-mime-types-to-extension/346169#346169) to loop around the check and retry with a new mime type array.

For us, we regularly hit this when users attempt to submit drawings of various types through a gravity forms contact form.

#82 @bobbingwide
4 years ago

Lack of progress on this is blocking an issue I raised in Gutenberg. https://github.com/WordPress/gutenberg/issues/23510

I suggested a simple workaround for that issue in a comment to TRAC #46775

#83 follow-up: @quangthien27
3 years ago

"potx" is still not able to upload, as its mime has been changed to "application/vnd.openxmlformats-officedocument.presentationml.presentation", not "application/vnd.openxmlformats-officedocument.presentationml.template" as per in /wp-includes/functions.php, "wp_get_mime_types" function

#84 in reply to: ↑ 83 @blobfolio
3 years ago

Replying to quangthien27:

"potx" is still not able to upload, as its mime has been changed to "application/vnd.openxmlformats-officedocument.presentationml.presentation", not "application/vnd.openxmlformats-officedocument.presentationml.template" as per in /wp-includes/functions.php, "wp_get_mime_types" function

Office document extensions and media types do not have a one-to-one relationship. Some potx files will have the media type application/vnd.openxmlformats-officedocument.presentationml.template, others won't.

There are workarounds for this in the Lord of the Files plugin. If you install that, you should be able to upload potx again.

#85 @costdev
12 months ago

#55939 was marked as a duplicate.

This ticket was mentioned in Slack in #core-media by zodiac1978. View the logs.


12 months ago

#87 @oglekler
7 months ago

I tried to upload potx in 6.4-alpha-56267-src and this issue is still valid.

#88 @hellofromTonya
5 months ago

#59277 was marked as a duplicate.

Note: See TracTickets for help on using tickets.