Bug 537120 (CVE-2010-1197)

Content-Disposition: attachment ignored if Content-Type: multipart also present

VERIFIED FIXED in mozilla1.9.3a5

Status

Core Graveyard
File Handling
VERIFIED FIXED
7 years ago
10 months ago

People

(Reporter: bsterne, Assigned: bz)

Tracking

({fixed1.9.0.20, verified1.9.1, verified1.9.2})

unspecified
mozilla1.9.3a5
fixed1.9.0.20, verified1.9.1, verified1.9.2
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 beta1+, blocking1.9.2 .4+, status1.9.2 .4-fixed, status1.9.1 .10-fixed)

Details

(Whiteboard: [sg:moderate][xss against sites who allow users to set C-T but who rely on C-D: attachment for protection])

Attachments

(3 attachments)

(Reporter)

Description

7 years ago
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.
Component: File Handling → File Handling
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.
(Reporter)

Comment 2

7 years ago
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?
(Reporter)

Comment 4

7 years ago
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
blocking1.9.2: ? → .3+
jst/bz - it's been four weeks :) any chance of this making an early-week code freeze? I'm guessing no.
Blocks: 344278
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] [diff] [review]
Part 1: make sure multipart parts install a type sniffer
Attachment #438135 - Flags: review?
Attachment #438135 - Flags: review? → review?(jduell.mcbugs)
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.
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
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Attachment #438135 - Flags: approval1.9.2.4?
Attachment #438135 - Flags: approval1.9.1.10?

Updated

7 years ago
Attachment #438135 - Flags: approval1.9.2.4?
Attachment #438135 - Flags: approval1.9.2.4+
Attachment #438135 - Flags: approval1.9.1.10?
Attachment #438135 - Flags: approval1.9.1.10+

Comment 13

7 years ago
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.
Attachment #438136 - Flags: approval1.9.2.4?
Attachment #438136 - Flags: approval1.9.1.10?

Updated

7 years ago
Attachment #438136 - Flags: approval1.9.2.4?
Attachment #438136 - Flags: approval1.9.2.4+
Attachment #438136 - Flags: approval1.9.1.10?
Attachment #438136 - Flags: approval1.9.1.10+
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.3a5
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
status1.9.2: --- → .4-fixed
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.
Status: RESOLVED → VERIFIED
status1.9.1: --- → .10-fixed
Keywords: checkin-needed
Flags: in-testsuite?
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).
Keywords: verified1.9.1, verified1.9.2
Alias: CVE-2010-1197
Group: core-security
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 1.9.0.20, 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 1.9.0.20, 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
Keywords: fixed1.9.0.20
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.