Uploading a malformed file

[RamyTalal]

It is possible to disguise random content as .gif.

To reproduce, create a file named file.gif with the following contents:

<?xml version="1.0" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"><svg onload="alert(hello)" xmlns="http://www.w3.org/2000/svg"><defs><font id="x"><font-face font-family="y"/></font></defs></svg>

<!--vector credits to author. -->

or just

alert('hello');

My config: http://bin.fuelphp.com/snippet/view/LL

If I remove image/gif from the whitelist then the file is denied.

https://github.com/fuelphp/upload/blob/master/src/File.php#L235 is causing the bug.

[WanWizard]

Why is that causing the bug?

What that line does is say "if the mimetype is not detected by fileinfo but is generic (either text/plain or application/octet-stream), use the type determined by PHP". Worse case it is the same, but it might be better.

So what is in your test the value of 'type' and 'mimetype'?

[RamyTalal]

Mimetype is text/plain and type is image/gif. I whitelisted image/gif but not text/plain.

Maybe add a config item for strict detection?

[WanWizard]

So both are wrong? Because you said you were uploading an svg file?

Or is that your test file which contains text (xml)? In which case the mimetype is correct and PHP detection is wrong...

[RamyTalal]

I can put any text plain/text in the file and it validates as the extension.

The PHP detection is wrong, but the fileinfo is correct.

[WanWizard]

yeah, because PHP assumes (based incorrectly on the extension I think) that it's an "image/gif", which as I understand it you have whitelisted.

The code is written in the assumption that either PHP or fileinfo would have the correct mimetype, and it picks the most specific. Which in your case is clearly the wrong one.

Have to think on how to approach this, because it is not always the case, so simply removing this code will break other uploads...

[RamyTalal]

Maybe add a strict setting?

[WanWizard]

That doesn't fix it.

In your test case, the correct mimetype is "text/xml", which isn't detected by either method. In other cases, PHP is right, fileinfo is wrong, or the otherway around.

So the trick is how to always pick the correct one. Until now, the assumption was taken that if one was generic (text/plain or application/octet-stream), the other would be more detailed and correct. In your test, it is shown to be more detailed, but not correct at all...

[RamyTalal]

I will do more tests tomorrow, I'll keep you posted.

[RamyTalal]

I've done some tests and fileinfo seems to be correct.

image/svg+xml:

<?xml version="1.0" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"><svg onload="alert(ManGoMAn)" xmlns="http://www.w3.org/2000/svg"><defs><font id="x"><font-face font-family="y"/></font></defs></svg>

<!--vector credits to author. -->

text/plain (notice the blank line above the xml tag):

<?xml version="1.0" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"><svg onload="alert(ManGoMAn)" xmlns="http://www.w3.org/2000/svg"><defs><font id="x"><font-face font-family="y"/></font></defs></svg>

<!--vector credits to author. -->

The PHP detection returned image/gif for both. My PHP version is 5.6.15.

[WanWizard]

Try uploading something Microsoft, like a docx file. That used to return "application/zip", which is technically true, but rather useless. And I can remember that in this case, PHP had it right. Not sure things have changed since then, I guess it depends on the quality of your mimes file...

[RamyTalal]

If I rename a zip file to .docx I get application/zip with fileinfo and PHP.

[WanWizard]

That is expected, I meant the other way around, if you upload a real docx document. Here's a test file if you don't have one: https://www2.le.ac.uk/Members/davidwickins/old/test.docx/view

(sorry to ask you this, don't have a setup at hand where I can test it myself atm)

[WanWizard]

The correct mimetype should be application/vnd.openxmlformats-officedocument.wordprocessingml.document

See http://blogs.msdn.com/b/vsofficedeveloper/archive/2008/05/08/office-2007-open-xml-mime-types.aspx

[RamyTalal]

I get application/vnd.openxmlformats-officedocument.wordprocessingml.document with fileinfo and application/zip with PHP.

[WanWizard]

Ok, cool, looks like fileinfo has picked up it's pace and the fallback can safely be removed then. Thanks very much for veryfying this!

[RamyTalal]

Glad to help!