579744 - (CVE-2010-2768) UTF-7 Universal XSS by overriding document charset using <object> type attribute

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David Lin-Shung Huang]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.99 Safari/533.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6 ( .NET CLR 3.5.30729)

In Firefox, the "type" attribute of an <object> tag can override the charset of a framed HTML document, even if the document is included across origins. The charset specified in the "type" attribute overrides any charset of the document that is specified using <?xml encoding="utf-8"?>, <meta charset="utf-8">, or <meta http-equiv="Content-Type" value="text/html;charset=utf-8">. This allows an attacker to bypass server XSS filtering of angle brackets and inject arbitrary JavaScript code encoded in UTF-7 into web sites. It works on web sites that do not specify a charset in the Content-Type header (416 out of the Alexa top 1000). It does not work on web sites that specify a charset in the Content-Type header (584 out of the Alexa top 1000).


Reproducible: Always

Steps to Reproduce:
Here's a proof of concept exploit, using bankofthewest.com as an example:
<object type="text/html; charset=UTF-7" data="https://srch01.bankofthewest.com/search?q=%2BACI-%2BAD4-%2BADw-script%2BAD4-alert(document.location)%2BADw-/script%2BAD4-"></object>
Actual Results:  
Bank of the West attempts to set the charset using a meta tag, but Firefox ignores it and parses the response document as UTF-7. The exploit is successful.

Expected Results:  
Firefox should not let the "type" attribute of a cross-origin <object> tag influence the charset of the rendered object.

See also: bug 356280, bug 406777, bug 408457, bug 530647, bug 414064

Comment 1

[Zack Weinberg (:zwol)]

I'm not sure exactly where to file this, but I think DOM Core/HTML is at least going to get more attention than General.  I see that the usual suspects are already cc:ed.

Comment 2

[Boris Zbarsky [:bzbarsky]]

So the issue here is that SetContentType before AsyncOpen sets not just a type hint but also a charset hint.  If the HTTP response doesn't have a charset parameter, we then use this hint as the HTTP-level charset.  And the HTTP-level charset overrides <meta> tags and the like.

This is reasonably easy to work around in the <object> code, but I think the necko behavior here is _really_ unintuitive.  I'd prefer that SetContentType just set the type and ignore any charset param on the given type, or take a boolean for whether to set the charset or something... Otherwise these things will keep popping up.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note that we have a similar issue with:

<a type="text/html; charset=UTF-7" href="https://srch01.bankofthewest.com/search?q=%2BACI-%2BAD4-%2BADw-script%2BAD4-alert(document.location)%2BADw-/script%2BAD4-">Click me</a>

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

I don't think I can do the parsing up front in nsObjectLoadingContent because at least some parts of the dispatch to plug-ins actually depend on type parameters (I'm thinking especially for Java here), as I recall...  Hence the parsing right before the SetContentType callsites.

Comment 5

[Boris Zbarsky [:bzbarsky]]

This needs to block (if nothing else so I can check it in).

Comment 6

[Daniel Veditz [:dveditz]]

Do we need separate branch patches? If not go ahead and request approval on this one.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 458549 [details] [diff] [review]
Fix

Looks like this applies cleanly to 1.9.2.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.9.1 merge; just had to s/nsDocShell/nsWebShell/, basically

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 458549 [details] [diff] [review]
Fix

I guess I need trunk approval too...

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 458549 [details] [diff] [review]
Fix

a=beltzner, please remember that (AIUI) this will need to be landed on the security-repo until we're ready to land it on the branches.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 458549 [details] [diff] [review]
Fix

Approved for 1.9.2.8, a=dveditz

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 459116 [details] [diff] [review]
1.9.1 merge; just had to s/nsDocShell/nsWebShell/, basically

Approved for 1.9.1.12, a=dveditz for release-drivers

Comment 13

[Boris Zbarsky [:bzbarsky]]

> this will need to be landed on the security-repo

That doesn't exist yet.  I'm just going to land it on m-c and the branches today.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Pushed:
  http://hg.mozilla.org/mozilla-central/rev/9d16542a1af2
  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ad35ca567289
  http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3e8356d36728

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Can someone explain how to turn the PoC information in comment 0 into STR for bug verification?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Paste the given string into a file called test.html and then load the file.  If you see alerts, you're seeing this bug.

For comment 3, paste the string into an HTML file, load the file, and then click the link.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Boris, I did that already and saw no alerts in 3.6.8 release on Windows XP. I saw, literally, nothing. Blank window with invisible object tag.

Same for comment 3 except clicking on the link gave me a security certificate warning because it isn't from a trusted site.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Odd.  I can reproduce the bug on Mac with no trouble.

Comment 19

[Boris Zbarsky [:bzbarsky]]

In 3.6.8, that is.

Comment 20

[Al Billings [:abillings - ex-MoCo]]

It turns out that wrapping the object in tags is bad for repro'ing. Once I removed those, it repro'd cleanly.

Verified the bug on and 1.9.2.8 on XP.

Verified fixed for 1.9.2.9 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9pre) Gecko/20100811 Namoroka/3.6.9pre ( .NET CLR 3.5.30729).

With 1.9.1.11, I cannot reproduce the issue. No alerts at all. Are we sure it reproduces there?

Comment 21

[Martin Stránský [:stransky] (ni? me)]

Attachment: 1.8.0 version

Comment 23

[Zack Weinberg (:zwol)]

Does the fix in this bug cover <embed>, <iframe>, <script>, <link>, and all the other HTML tags that can take a type= attribute?

Comment 24

[Boris Zbarsky [:bzbarsky]]

It covers <embed>.

<iframe> doesn't take a type= attribute.

The type= attribute of <script> is never passed to an nsIChannel SetContentType.

The type attribute of <input> means something totally different.

The type attribute of <link> is not passed to an nsIChannel SetContentType.

But in any case, this bug mattered because you could change how something that could include scripts inline is parsed.  In practice, that means only documents, right?

Comment 25

[Zack Weinberg (:zwol)]

(In reply to comment #24)
> But in any case, this bug mattered because you could change how something that
> could include scripts inline is parsed.  In practice, that means only
> documents, right?

<link rel=stylesheet> admits at least one attack which is more powerful if the attacker can force UTF-7 from the containing document (bug 524223) and I wouldn't be surprised if you could gin up something analogous with <script>.  So I'd want to be very sure indeed that a charset= indicator in the type attribute of those tags was in fact ignored.

Comment 26

[Boris Zbarsky [:bzbarsky]]

charset="" in the type attribute is ignored.  But the charset="" attribute is not ignored, of course.

Comment 27

[Zack Weinberg (:zwol)]

Your "of course" worries me.

Comment 28

[Zack Weinberg (:zwol)]

To elaborate: do we have an "only pay attention to this if same-domain" check on the charset= attributes of <link> and <script>?  If not, perhaps we need to add one.

Comment 29

[Boris Zbarsky [:bzbarsky]]

We do not.  I'd love a separate bug that explains why such a check would be beneficial!

Comment 30

[Zack Weinberg (:zwol)]

(In reply to comment #29)
> We do not.  I'd love a separate bug that explains why such a check would be
> beneficial!

Filed bug 603740.  Let me know if the explanation is not clear or convincing.