368700 - Using TLDs in document.domain should not be allowed

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Wladimir Palant]

I recently discovered XSS vulnerabilities in myspace.com and live.com/msn.com that could be abused because these sites don't check properly what they write into document.domain. The attack requires a malicious page on a .com domain, it can set its document.domain to "com" and make a vulnerable site loaded in a frame do the same. A variation of this attack is setting document.domain to "com." (you have to use addresses like http://www.myspace.com./ then).

Internet Explorer prevents the first attack, from what I can tell it doesn't allow changing document.domain to something without a dot in it. As to the second attack, exploitation is more difficult in Internet Explorer and Opera since those treat "myspace.com" and "myspace.com." as different domains when it comes to cookies. This makes users of Gecko-based browsers the most vulnerable ones.

While the web sites are certainly at fault here, this issue isn't a rare one according to Google's CodeSearch. And we could fix it relatively easily - don't allow using TLDs in document.domain. Setting document.domain to "localhost" for local testing will still be possible because the value doesn't change. I should have a patch soon.

Comment 1

[Wladimir Palant]

Attachment: Testcase

This testcase shouldn't be able to set document.domain to "org" or "org." (the latter when opened as https://bugzilla.mozilla.org./attachment.cgi?...).

Comment 2

[Wladimir Palant]

Attachment: Proposed patch

This together with the patch in bug 368702 should take care of the issue.

Comment 3

[Robert Chapin]

Related to Bug #252342 ?

Comment 4

[Wladimir Palant]

How so? Cookie domain checks are a different thing, even though they also require the Effective TLD Service for a correct implementation.

Comment 5

[Robert Chapin]

Same issue, in my mind ;)  The guys working on the cookies should have audited all related domain trust issues.  It would be sad to find "Uber certificate signed for .com. valid on entire TLD" or similar bugs that are foreseeable because they need to rely on the same service.  (bad analogy, but you get my point?)  btw, I hope this vulnerability is caused by a CNAME/javabug rather than script injection.

Comment 6

[Robert Chapin]

Oops also, "From what I can tell it doesn't allow changing document.domain to something without a dot in it."  Does that mean the attack works just fine on vulnerable co.uk websites?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 253353 [details] [diff] [review]
Proposed patch

>Index: content/html/document/src/nsHTMLDocument.cpp
>+      nsCAutoString str;
>+      PRUint32 tldLength;
>+
>+      CopyUTF16toUTF8(aDomain, str);

Why not:

  NS_ConvertUTF16toUTF8 str(aDomain);

?

(NS_FAILED(tldService->GetEffectiveTLDLength(str, &tldLength)))
>+        return NS_ERROR_FAILURE;

Why not return whatever GetEffeciveTLDLength returned instead?  That would make more sense to me.

With those nits, looks good.

Comment 8

[Wladimir Palant]

Attachment: Proposed patch with nits fixed

Fixed bz's nits, also added a missing null-check in case getting EffectiveTLDService fails.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 260452 [details] [diff] [review]
Proposed patch with nits fixed

>+      if (!tldService)
>+        return NS_ERROR_FAILURE;

ObNit: NS_ERROR_NOT_AVAILABLE would be more appropriate

>+      nsresult rv = tldService->GetEffectiveTLDLength(str, &tldLength);
>+      if (tldLength < str.Length())

I hate this interface :-(

sr=dveditz

Comment 10

[Wladimir Palant]

Attachment: Patch to be checked in

Comment 11

[Nickolay_Ponomarev]

mozilla/content/html/document/src/nsHTMLDocument.cpp  3.718

Does this need a regression test?

Comment 12

[Wladimir Palant]

A regression test would be good, problem is to get proper testing if the only domain you can use in the test is localhost. nsIDocShell.loadStream would allow to associate our data with an arbitrary URL but it is [noscript].

Comment 13

[Brendan Eich [:brendan]]

At least one large portal contacted me expressing the desire for this patch to go into 1.8.0.x / Firefox 2.0.0.x soon.

/be

Comment 14

[Daniel Veditz [:dveditz]]

I'm not sure we can get the whole eTLD mechanism checked into the 1.8 branch (although then we could fix the cookie issue, too).

Comment 15

[Daniel Veditz [:dveditz]]

brendan: much as it would be great to land eTLD for the branch I'm not sure it's realistic to land at this time. Do you have a volunteer for the work who's not already swamped with FF3 stuff?

Comment 16

[Daniel Veditz [:dveditz]]

Firefox 3 will have this feature and is coming soon. Since it's not a critical security hole, new feature-ish, and potentially site-breaking (should any site be dumb enough to actually use this) it's hard to see how this is in scope for a branch release.

How is a large portal vulnerable? Wouldn't that mean that the site itself is setting document.domain to .com as well?

Comment 17

[Wladimir Palant]

(In reply to comment #16)
> How is a large portal vulnerable? Wouldn't that mean that the site itself is
> setting document.domain to .com as well?

Yes, they were writing a value into document.domain that they didn't validate properly. In case of MySpace, the algorithm was "take current host name and everything following the second last dot is our domain name" - only that you could use "www.myspace.com." as host name (no longer, will redirect to the version without a dot). In case of several Microsoft sites the case was even more primitive - they took a parameter from the query string and put it into document.domain without any validation whatsoever, obviously assuming that the browser will do enough validation already. Apparently, there isn't much awareness that setting document.domain to a random value is dangerous (probably because it shouldn't be - which was exactly the point of this bug).

Anyway, I guess this isn't worth porting to the branch at this time point - especially since we didn't even port effective TLDs yet.