Last Comment Bug 537120 - (CVE-2010-1197) Content-Disposition: attachment ignored if Content-Type: multipart also present
(CVE-2010-1197)
: Content-Disposition: attachment ignored if Content-Type: multipart also present
Status: VERIFIED FIXED
[sg:moderate][xss against sites who a...
: fixed1.9.0.20, verified1.9.1, verified1.9.2
Product: Core Graveyard
Classification: Graveyard
Component: File Handling (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla1.9.3a5
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
Depends on:
Blocks: 344278
  Show dependency treegraph
 
Reported: 2009-12-29 10:15 PST by Brandon Sterne (:bsterne)
Modified: 2016-06-22 12:16 PDT (History)
14 users (show)
bzbarsky: in‑testsuite?
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
beta1+
.4+
.4-fixed
.10-fixed


Attachments
testcase (load from mod_php server) (167 bytes, application/x-httpd-php)
2009-12-29 10:15 PST, Brandon Sterne (:bsterne)
no flags Details
Part 1: make sure multipart parts install a type sniffer (10.22 KB, patch)
2010-04-09 11:50 PDT, Boris Zbarsky [:bz] (still a bit busy)
jduell.mcbugs: review+
christian: approval1.9.2.4+
christian: approval1.9.1.10+
dveditz: approval1.9.0.next+
Details | Diff | Splinter Review
Part 2: change the URILoader to not do conversions for external handling (4.19 KB, patch)
2010-04-09 11:55 PDT, Boris Zbarsky [:bz] (still a bit busy)
jst: review+
christian: approval1.9.2.4+
christian: approval1.9.1.10+
dveditz: approval1.9.0.next+
Details | Diff | Splinter Review

Description Brandon Sterne (:bsterne) 2009-12-29 10:15:03 PST
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.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2009-12-29 11:23:40 PST
This is pretty similar to bug 344278, and happening for the same reasons.  See bug 344278 comment 9 for the analysis and possible solutions.
Comment 2 Brandon Sterne (:bsterne) 2010-02-01 14:01:27 PST
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.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2010-02-01 14:12:01 PST
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?
Comment 4 Brandon Sterne (:bsterne) 2010-03-01 08:03:44 PST
Just wanted to ping you guys again.  This is an externally reported bug and the reporter is asking about progress.
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2010-03-08 15:42:21 PST
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.
Comment 6 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-07 13:11:02 PDT
jst/bz - it's been four weeks :) any chance of this making an early-week code freeze? I'm guessing no.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2010-04-09 11:47:46 PDT
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
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2010-04-09 11:50:21 PDT
Created attachment 438135 [details] [diff] [review]
Part 1: make sure multipart parts install a type sniffer
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2010-04-09 11:55:12 PDT
Created attachment 438136 [details] [diff] [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 10 Johnny Stenback (:jst, jst@mozilla.com) 2010-04-09 16:03:13 PDT
Comment on attachment 438136 [details] [diff] [review]
Part 2: change the URILoader to not do conversions for external handling

Stealing review, r=jst
Comment 11 Jason Duell [:jduell] (needinfo me) 2010-04-10 01:57:56 PDT
Comment on attachment 438135 [details] [diff] [review]
Part 1: make sure multipart parts install a type sniffer

looks & tests good.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2010-04-10 09:48:54 PDT
Checked in on trunk:

http://hg.mozilla.org/mozilla-central/rev/31759ae87a88
http://hg.mozilla.org/mozilla-central/rev/fccdd48b82ae
Comment 13 christian 2010-04-12 11:43:58 PDT
Comment on attachment 438135 [details] [diff] [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.
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2010-04-12 17:00:29 PDT
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.
Comment 16 Al Billings [:abillings] 2010-04-13 16:00:47 PDT
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.
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2010-04-13 16:34:31 PDT
For moco internal testing you could use http://gandalf/~jst/content-disposition.php, other than that I don't know...
Comment 18 Al Billings [:abillings] 2010-04-13 16:55:45 PDT
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.
Comment 19 Al Billings [:abillings] 2010-04-13 17:20:35 PDT
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 20 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-18 22:00:37 PDT
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.
Comment 21 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-18 22:00:55 PDT
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.
Comment 22 Daniel Veditz [:dveditz] 2010-07-22 19:19:38 PDT
Comment on attachment 438135 [details] [diff] [review]
Part 1: make sure multipart parts install a type sniffer

Approved for 1.9.0.20, a=dveditz
Comment 23 Daniel Veditz [:dveditz] 2010-07-22 19:19:59 PDT
Comment on attachment 438136 [details] [diff] [review]
Part 2: change the URILoader to not do conversions for external handling

Approved for 1.9.0.20, a=dveditz
Comment 24 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-24 14:20:57 PDT
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

Note You need to log in before you can comment on or make changes to this bug.