Last modified: 2014-06-26 09:29:29 UTC
Created attachment 15612 [details] ZIP-type file with fake extension. In MimeMagic.php, there is a function that is supposed to detect zips - ::doGuessMimeType() - and zip types - ::detectZipType(). However, it fails when supplying small files like attached. Neither small ZIPs are detected nor other kind of zip-types. So I did some debugging and found that $tail in ::doGuessMimeType() appears to be empty when the file size is below about 8192 bytes. Lowering the number of bytes from the end of the file to set the cursor to in the call to fseek to the actual file size "fixes" the issue. Reproducible: * Production: Always. ** Evidence: When uploading c.jpg to Commons, the following response is issued: File extension ".jpg" does not match the detected MIME type of the file (unknown/unknown). ** Evidence: When uploading c_big.jpg to Commons, the following response is issued: File extension ".jpg" does not match the detected MIME type of the file (application/vnd.oasis.opendocument.spreadsheet). * Debug environment with PHP 5.4.20 (apache2handler): Always. ** Evidence: Debug logging and inspecting variables using xDebug and Eclipse.
Created attachment 15613 [details] This file will be detected by MediaWiki because $tail is not empty.
Cc ing csteipp. I suspect this might be exploitable...
Thanks Rainer / Brian, I'm going to put this into security while I play with it. I agree, that seems like it could be used for malicious purposes, but hopefully not.
Was it wrong to publicly submit a patch with I182128c6958244 ? How could I privately submit to Gerrit?
(In reply to Rainer Rillke @commons.wikimedia from comment #4) > Was it wrong to publicly submit a patch with I182128c6958244 ? How could I > privately submit to Gerrit? You can't. If its a super critical security issue, people usually put the patches on bugzilla I believe.
Adding -D (draft) to git review will keep it from being advertised, but gerrit doesn't have a good way to deal with private patches. This isn't horrible since the detection is still done when finfo is installed, or the wiki shells out for the detection. Also, since it's unknown/unknown, the file isn't allowed to be uploaded (unless $wgVerifyMimeType, in which case there are plenty of other ways to upload malicious files). So I'm not worried about this being public, although I think it's good hardening. But unless someone finds a way to get the upload to succeed with a relatively secure config, I don't think we need to be overly private about this bug.
Making this public again. I think we should also backport the fix, since it's a good security hardening measure.
Change 139070 had a related patch set uploaded by Rillke: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/139070
Change 139073 had a related patch set uploaded by Rillke: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/139073
Change 139074 had a related patch set uploaded by Rillke: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/139074
Release managers would be notified if Backport_to_Stable is set to "?", the docs say. Does this still work?
(In reply to Rainer Rillke @commons.wikimedia from comment #11) > Release managers would be notified if Backport_to_Stable is set to "?", the > docs say. Does this still work? The docs at https://www.mediawiki.org/wiki/Backporting_fixes state this and tarball release managers are aware of the flag; but there was no automatic bugmail notification sent to them so far, sigh. I've fixed this now.
There is no automatic method that I know of. We're still checking these manually. Cherry-picking, as was done here, is the best route to go because then it will be in the queue during our monthly maintenance releases. If that isn't possible, then adding us as CC on the bug at least sends us email.
Change 139070 merged by Mglaser: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/139070
Change 139074 merged by Mglaser: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/139074
Change 139073 merged by Mglaser: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/139073
Updating the flag from ? to + now that this has been backported.
Change 142041 had a related patch set uploaded by Mglaser: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/142041
Change 142041 merged by Mglaser: MimeMagic: Don't seek before BOF https://gerrit.wikimedia.org/r/142041
