Bugzilla@Mozilla – Bug 537120
Content-Disposition: attachment ignored if Content-Type: multipart also present
Last modified: 2010-07-24 14:20:57 PDT
Summon comment box
Created attachment 419432 [details] testcase (load from mod_php server) Ilja van Sprundel from IOActive reported this issue to security@m.o. When we receive both a C-D:attachment and a C-T:multipart header, we follow the C-T header and display the content inline. This could potentially lead to XSS if a site lets users specify Content-Type for a file but rely on C-D to serve the file as an attachment. Apparently, other browsers honor the C-D header. I tested in Chromium and that is the case. (filing as sec-sensitive for now) ------- Hey Guys, There's a small security bug in FF that would allow files that are offered as a download to be rendered as inline html instead: If Content-Disposition is set to attachment, but content-type is set to multipart (attacker controlled),then the content-disposition is ignored, and the content is seen as a mime header.if that mime header defines a content-type of text/html then it will be rendered inline. This allows for XSS on some webapplications. specifically webapplications that allow an attacker to upload a file, and set it's content-type, but where the webapp sets the content disposition to attachment, trying to force the user to download the file, instead of rendering the content inline (this is actually quite common, e.g. a webmail application, webapp allowing users to upload and download files, ...). Firefox appears to be the only browser that does this. IE, chrome and safari will honor the content-disposition and just offer it as a download. testes both on FF 3.0 and and 3.5. Regards, Ilja van Sprundel.
This is pretty similar to bug 344278, and happening for the same reasons. See bug 344278 comment 9 for the analysis and possible solutions.
Boris, any chance you can take a swing at the patch you proposed in bug 344278 comment 9? If you don't have time for this one, please reassign to someone appropriate.
I can do it if it's not needed any time in the next few weeks... and I'm not sure who has time offhand. jst, do you know?
Just wanted to ping you guys again. This is an externally reported bug and the reporter is asking about progress.
The people most suitable for working on this are unfortunately not available to work on this right now, but the plan is to have this fixed in time for 3.6.3. IOW, we should start seeing progress here in about 4 or so weeks.
jst/bz - it's been four weeks :) any chance of this making an early-week code freeze? I'm guessing no.
Still trying to figure out a way to write an automated test; in the meantime I put up a test at http://landfill.mozilla.org/ryl/testMultipart.cgi
Created attachment 438135 [details] [review] Part 1: make sure multipart parts install a type sniffer
Created attachment 438136 [details] [review] Part 2: change the URILoader to not do conversions for external handling This is safe because the only channels that can force external handling right now are HTTP and part channels, and they both do type sniffing themselves. There's an automated test for the multipart changes in part 1. I manually tested part 2 and filed bug 558412 on test infrastructure that would be needed for automated testing.
Comment on attachment 438136 [details] [review] Part 2: change the URILoader to not do conversions for external handling Stealing review, r=jst
Comment on attachment 438135 [details] [review] Part 1: make sure multipart parts install a type sniffer looks & tests good.
Checked in on trunk: http://hg.mozilla.org/mozilla-central/rev/31759ae87a88 http://hg.mozilla.org/mozilla-central/rev/fccdd48b82ae
Comment on attachment 438135 [details] [review] Part 1: make sure multipart parts install a type sniffer a=LegNeato for 1.9.2.4 and 1.9.1.10 Please note code freeze for 1.9.2.4 is today @ 11:59 pm PST.
Fixed for 1.9.2 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7e76b3e0159d http://hg.mozilla.org/releases/mozilla-1.9.2/rev/de273d70e79b
Fixed for 1.9.1 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/abc92619654c http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f9e9ac4016a7 Also verified on mozilla-central, 1.9.2 and 1.9.1.
Do we have a web server with mod_php installed anywhere? I thought that mine had it installed but it doesn't and I don't have root on that box.
For moco internal testing you could use http://gandalf/~jst/content-disposition.php, other than that I don't know...
Johnny, can you give me the internal IP of that machine? I can't resolve it with just the single name when I VPN into the net.
I have it now. Post-fix, the user is prompted to download the file appropriately. I verified it for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4pre) Gecko/20100413 Namoroka/3.6.4pre (.NET CLR 3.5.30729). For 1.9.1, I used Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.10pre) Gecko/20100413 Shiretoko/3.5.10pre (.NET CLR 3.5.30729).
Comment on attachment 438135 [details] [review] Part 1: make sure multipart parts install a type sniffer Requesting approval1.9.0.next on this patch so that we can take it in upcoming Camino 2.0.x security and stability updates. If approved, I'll handle the checkins, unless the patch author requests otherwise.
Comment on attachment 438136 [details] [review] Part 2: change the URILoader to not do conversions for external handling Requesting approval1.9.0.next on this patch so that we can take it in upcoming Camino 2.0.x security and stability updates. If approved, I'll handle the checkins, unless the patch author requests otherwise.
Comment on attachment 438135 [details] [review] Part 1: make sure multipart parts install a type sniffer Approved for 1.9.0.20, a=dveditz
Comment on attachment 438136 [details] [review] Part 2: change the URILoader to not do conversions for external handling Approved for 1.9.0.20, a=dveditz
Part 1: Checking in netwerk/streamconv/converters/nsMultiMixedConv.cpp; /cvsroot/mozilla/netwerk/streamconv/converters/nsMultiMixedConv.cpp,v <-- nsMultiMixedConv.cpp new revision: 1.96; previous revision: 1.95 done Checking in netwerk/streamconv/converters/nsMultiMixedConv.h; /cvsroot/mozilla/netwerk/streamconv/converters/nsMultiMixedConv.h,v <-- nsMultiMixedConv.h new revision: 1.29; previous revision: 1.28 done RCS file: /cvsroot/mozilla/netwerk/test/unit/test_multipart_streamconv.js,v done Checking in netwerk/test/unit/test_multipart_streamconv.js; /cvsroot/mozilla/netwerk/test/unit/test_multipart_streamconv.js,v <-- test_multipart_streamconv.js initial revision: 1.1 done Part 2: Checking in uriloader/base/nsURILoader.cpp; /cvsroot/mozilla/uriloader/base/nsURILoader.cpp,v <-- nsURILoader.cpp new revision: 1.151; previous revision: 1.150 done