Using TLDs in document.domain should not be allowed

RESOLVED FIXED in mozilla1.9alpha4



11 years ago
10 years ago


(Reporter: Wladimir Palant, Assigned: Wladimir Palant)


Dependency tree / graph
Bug Flags:
blocking1.8.1.12 -
blocking1.8.1.13 -
blocking1.8.1.15 -
wanted1.8.1.x -
in-testsuite ?

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [sg:want P2])


(2 attachments, 2 obsolete attachments)



11 years ago
I recently discovered XSS vulnerabilities in and 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 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 "" and "" 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.


11 years ago
Depends on: 331510
No longer depends on: 342314


11 years ago
Depends on: 368702

Comment 1

11 years ago
Created attachment 253350 [details]

This testcase shouldn't be able to set document.domain to "org" or "org." (the latter when opened as
Assignee: dveditz → trev

Comment 2

11 years ago
Created attachment 253353 [details] [diff] [review]
Proposed patch

This together with the patch in bug 368702 should take care of the issue.
Attachment #253353 - Flags: review?(darin.moz)

Comment 3

11 years ago
Related to Bug #252342 ?

Comment 4

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

Comment 5

11 years ago
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

11 years ago
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 websites?


11 years ago
Attachment #253353 - Flags: review?(darin.moz) → review?(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.
Attachment #253353 - Flags: review?(bzbarsky) → review+
Flags: blocking1.9?
Whiteboard: [sg:want P2]

Comment 8

11 years ago
Created attachment 260452 [details] [diff] [review]
Proposed patch with nits fixed

Fixed bz's nits, also added a missing null-check in case getting EffectiveTLDService fails.
Attachment #253353 - Attachment is obsolete: true
Attachment #260452 - Flags: superreview?(jst)
Attachment #260452 - Flags: review+
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 :-(

Attachment #260452 - Flags: superreview?(jst) → superreview+

Comment 10

11 years ago
Created attachment 260511 [details] [diff] [review]
Patch to be checked in
Attachment #260452 - Attachment is obsolete: true


11 years ago
Whiteboard: [sg:want P2] → [sg:want P2] [checkin needed]

Comment 11

11 years ago
mozilla/content/html/document/src/nsHTMLDocument.cpp  3.718

Does this need a regression test?
Last Resolved: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [sg:want P2] [checkin needed] → [sg:want P2]
Target Milestone: --- → mozilla1.9alpha4

Comment 12

11 years ago
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].
Flags: blocking1.9?
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.

Flags: blocking1.8.1.11?
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).
Flags: wanted1.8.1.x+
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?
Flags: blocking1.8.1.13?
Flags: blocking1.8.1.12?
Flags: blocking1.8.1.12-
Flags: blocking1.8.1.14?
Flags: blocking1.8.1.13?
Flags: blocking1.8.1.13-
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?
Flags: wanted1.8.1.x-
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.15?
Flags: blocking1.8.1.15-

Comment 17

10 years ago
(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 "" 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.
You need to log in before you can comment on or make changes to this bug.