Closed Bug 655649 (CVE-2012-3985) Opened 13 years ago Closed 12 years ago

Script access checks should use effective script origin, not origin

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16
Tracking Status
firefox5 - wontfix
firefox6 - wontfix
firefox7 - wontfix
firefox8 + wontfix
firefox9 --- wontfix
firefox10 --- wontfix
firefox11 --- affected
firefox12 --- affected
firefox13 --- affected
firefox16 --- fixed
firefox-esr10 --- wontfix
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: mozilla, Assigned: bholley)

References

Details

(Keywords: regression, sec-moderate, Whiteboard: [sg:moderate][advisory-tracking+] (sg:high against sites using document.domain)[cpg])

Attachments

(7 files)

The HTML5 spec requires that browsers raise a SECURITY_ERR exception when members of the Window object (e.g. localStorage) are accessed by scripts that have a different effective script origin. Firefox allows access by scripts that have the same origin OR the same effective script origin.

Here's a scenario where the new behavior has an observable security impact:

https://attacker.cmu.edu/ wants to access the localStorage of https://weblogin.cmu.edu/. Assume that there exists a page at https://help.weblogin.cmu.edu/ that sets document.domain = 'cmu.edu' and that https://weblogin.cmu.edu/ sets its document.domain to 'weblogin.cmu.edu'.

In Chrome/Safari/IE8, I claim that the attacker can't do it. They can inject script into the one help.weblogin.cmu.edu page that set document.domain to 'cmu.edu' but this page can't script other help.weblogin.cmu.edu pages nor can it script weblogin.cmu.edu.

In Firefox 4, the attack works. The frame hierarchy looks like this:

https://attacker.cmu.edu/
   ---> https://help.weblogin.cmu.edu/
       ---> https://help.weblogin.cmu.edu/blah
           ---> https://weblogin.cmu.edu/

The top frame can script the second frame by setting document.domain to 'cmu.edu', the second frame can script the third due to Firefox's non-standard access check, the third frame can script the fourth by setting its document.domain to 'weblogin.cmu.edu'

There may be less contrived scenarios; this is what I could come up with after thinking about it for a few minutes. I'm setting the security flag on this bug because I haven't thought through all the ramifications of the new origin enforcement behavior, but it's fine to clear it if you feel the risk is low.

Incidentally, this attack also worked in IE7, but for a different reason. IE7 allows 'help.weblogin.cmu.edu' to set its document.domain to 'weblogin.cmu.edu' after setting it to 'cmu.edu'. This was fixed in IE8, which now prohibits setting document.domain to a longer value than its current value.

http://www.adambarth.com/papers/2009/barth-weinberger-song.pdf has a great explanation of how to do the HTML5 script access check quickly. The key is to get the access check down to a single pointer comparison in the common case.
I believe this was a purposeful decision when the current compartment system was implemented.  The reasoning was that if two pages were once same-origin then making them different origins at a later time doesn't buy you much because you could have already injected whatever you want from the one page into the other (e.g. an onmessage handler that evals the passed-in string).  The effect is that document.domain changes can _drop_ security restrictions in Gecko 2.0 but not _introduce_ them, which is what this bug seems to be about.

That reasoning does fall down in the example described in comment 0, I think: there is no point in time when https://help.weblogin.cmu.edu/ can both inject script into https://help.weblogin.cmu.edu/blah and have script injected from https://attacker.cmu.edu/ in a system which allows document.domain to introduce security restrictions.
Component: Security: CAPS → XPConnect
QA Contact: caps → xpconnect
Depends on: 601277
Are you planning to propose this new security model for the web to the HTML working group?  If the working group decides to keep the current model, are you planning to continue to implement a non-standards compliant security model?  That seems very bad for the web.
I can't speak for Mozilla's strategy, but we rarely intentionally diverge from standards, except if the standard is clearly wrong and there is consensus by implementers to ignore it. It might be worth discussing this change with the WG but for now I don't think anyone assumes that the current model is clearly wrong. Whether this is really that bad for the web I am a bit doubtful about. Extensive document.domain is not all that common, and it will likely get less over time. We are currently plotting out our approach to fix this. We will either lazily split compartments (brain transplant of document graphs), or use zones to keep each window in its own compartment.
Yeah, there was a bit of "laziness" on our part in making this decision. In particular, when we were implementing the original compartments stuff, we couldn't fix this bug as filed in an easy way (lazily splitting compartments wasn't possible until just before Firefox 4). We're definitely planning on fixing things to revoke access on document.domain setting.
Whose bug is this? Blake, Andreas?

/be
(In reply to comment #1)
> The effect is that document.domain changes can _drop_ security restrictions
> in Gecko 2.0 but not _introduce_ them, 

As broken as document.domain is in the first place it's not a good idea to change its historical behavior. That's just laying traps for unwary web app developers to fall into.

There's a reason the document.domain spec doesn't allow pages to re-lengthen document.domain back to the original domain once it's been truncated. Once a page has changed its document.domain it's tainted and shouldn't be allowed to spread that taint by rejoining its initial origin.
Keywords: regression
Whiteboard: [sg:moderate] (sg:high against sites using document.domain)
--> Andreas because it sounds like he needs to make a call on which approach to take.

We've diverged from the spec and this can put some sites at risk, we need to fix it. Seems unlikely to be fixed safely in Firefox 5 which is almost done, but we'd like to get this in Firefox 6.
Assignee: nobody → gal
From a code standpoint, the safest milestone is either Firefox 7 or 8. We're targeting compartment-per-window for Firefox 7, which makes the fix for this bug a lot easier (brain transplanting proxies is always safer than brain transplanting wrapped natives).
Whiteboard: [sg:moderate] (sg:high against sites using document.domain) → [sg:moderate] (sg:high against sites using document.domain) [probably not 'till 7, see #8]
We're up against the endgame in 6 and if we're not going to be doing anything here for 6, we probably don't need to keep tracking. Dveditz, what do you think?
Given Blake's comment above we won't be able to fix this for 6, tracking for 7, but I'm guessing the change will be invasive enough to port there too, so this might be pushed further...
Blake, can you take this? I don't think Andreas will have time to focus on this any time soon.
Depends on: cpg
Whiteboard: [sg:moderate] (sg:high against sites using document.domain) [probably not 'till 7, see #8] → [sg:moderate] (sg:high against sites using document.domain)[cpg]
Could you please CC me on bug 729150? Thanks
I have patches for this. Coming right up. I'm going to flag bill for review on the nitty-gritty of the transplant code, since he's probably fresher on it than Blake and has a lower review burden.
Aside from some renaming, no functionality has been changed.
Attachment #637480 - Flags: review?(wmccloskey)
Simple rename/move. No changes other than renaming some parameters.
Attachment #637481 - Flags: review?(wmccloskey)
Now that we're not doing that awful Location thing anymore, I think we should assert this. v1
Attachment #637482 - Flags: review?(wmccloskey)
Now that we have nsExpandedPrincipal, the current way of doing things is wrong. For some reason, the old document.domain hackery was hiding the failures here.
Attachment #637485 - Flags: review?(mrbkap)
Note that the tests for this are over in bug 601277.
FWIW I also pushed this to try earlier. The only bustage was in the nsExpandedPrincipal xpcshell tests, which prompted part 6 of the patch stack here.

https://tbpl.mozilla.org/?tree=Try&rev=3fc46f385017
CCing gabor so that he knows about patch 6.