search

silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 65 forks source link

FIX Allow inconclusive mimetypes #582

Closed GuySartorelli closed 6 months ago

GuySartorelli commented 6 months ago

I don't want to bump the dependency in a patch release so we can't use a named argument for the new useInconclusiveMimeTypeFallback parameter since it isn't available in flysystem until 3.23.0.

Passing an extra argument doesn't cause errors so it's safe if the project doesn't have flysystem updated - but passing missing named arguments does cause errors.

Issue

GuySartorelli commented 6 months ago

It tries to get a mimetype from the file contents - but even if it gets a mimetype and it's a valid mimetype, it will ignore that type if it's deemed to be "inconclusive". If then checks the file name - and if it can't get any mimetype from the file name (inconclusive or otherwise), an exception is thrown.

So for .brf files for example, the file contents look like text/plain. That's considered inconclusive, so it moves on to check the file type, and there is no registered mime type for .brf in flysystem, so it throws an exception.

If it returned text/plain (which this PR will make it do), that mimetype is still validated afterwards. In the case of .brf files, the mimevalidator was updated with the correct mime type, which is one of text/plain or application/octet-stream. If the mime type detected isn't one of those, it'll still fail validation.

So long as the mimetype validator is set up correctly for a given file type (which AFAIK is required to include it as uploadable) there is no risk of vulnerabilities of that type.