Closed Bug 537120 (CVE-2010-1197) Opened 11 years ago Closed 11 years ago
Content-Disposition: attachment ignored if Content-Type: multipart also present
167 bytes, application/x-httpd-php
10.22 KB, patch
|Details | Diff | Splinter Review|
4.19 KB, patch
|Details | Diff | Splinter Review|
Ilja van Sprundel from IOActive reported this issue to firstname.lastname@example.org. 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.
Product: Firefox → Core
QA Contact: file.handling → file-handling
Version: Trunk → unspecified
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.
Assignee: nobody → bzbarsky
Whiteboard: [sg:moderate][xss against sites who allow users to set C-T but who rely on C-D: attachment for protection]
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.
blocking1.9.2: --- → ?
blocking2.0: --- → beta1
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
Attachment #438135 - Flags: review? → review?(jduell.mcbugs)
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.
Attachment #438136 - Flags: review?(jduell.mcbugs)
Comment on attachment 438136 [details] [diff] [review] Part 2: change the URILoader to not do conversions for external handling Stealing review, r=jst
Attachment #438136 - Flags: review?(jduell.mcbugs) → review+
Comment on attachment 438135 [details] [diff] [review] Part 1: make sure multipart parts install a type sniffer looks & tests good.
Attachment #438135 - Flags: review?(jduell.mcbugs) → review+
Checked in on trunk: http://hg.mozilla.org/mozilla-central/rev/31759ae87a88 http://hg.mozilla.org/mozilla-central/rev/fccdd48b82ae
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 438135 [details] [diff] [review] Part 1: make sure multipart parts install a type sniffer a=LegNeato for 184.108.40.206 and 220.127.116.11 Please note code freeze for 18.104.22.168 is today @ 11:59 pm PST.
Target Milestone: --- → mozilla1.9.3a5
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:22.214.171.124pre) 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:126.96.36.199pre) Gecko/20100413 Shiretoko/3.5.10pre (.NET CLR 3.5.30729).
Comment on attachment 438135 [details] [diff] [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.
Attachment #438135 - Flags: approval1.9.0.next?
Comment on attachment 438136 [details] [diff] [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.
Attachment #438136 - Flags: approval1.9.0.next?
Comment on attachment 438135 [details] [diff] [review] Part 1: make sure multipart parts install a type sniffer Approved for 188.8.131.52, a=dveditz
Attachment #438135 - Flags: approval1.9.0.next? → approval1.9.0.next+
Comment on attachment 438136 [details] [diff] [review] Part 2: change the URILoader to not do conversions for external handling Approved for 184.108.40.206, a=dveditz
Attachment #438136 - Flags: approval1.9.0.next? → approval1.9.0.next+
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
You need to log in before you can comment on or make changes to this bug.