Bug 1105045 (CVE-2015-4502)

Receiver passed to proxy get hook is not outerized when proxy is on the window's proto chain

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: bzbarsky, Assigned: jorendorff)

Tracking

({sec-moderate})

unspecified
mozilla43
x86
macOS
Points:
---
Bug Flags:
sec-bounty +
in-testsuite +

Firefox Tracking Flags

(firefox40 wontfix, firefox41 fixed, firefox42 fixed, firefox43 fixed, firefox-esr38 wontfix, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.1S unaffected, b2g-v2.2 unaffected, b2g-v2.2r unaffected, b2g-master fixed)

Details

(Whiteboard: [reporter-external][post-critsmash-triage][adv-main41+])

Attachments

(2 attachments)

Posted file Testcase
Credit for finding this goes to André Bargull (cced).

If you have a proxy that has a handler like this:

  {
    get: function(t,pk,r) {
    },
    has: function(t,pk) {
      return pk == 'try_to_get_global_object';
    }
  }

on the proto chain of the window and then you evaluate the bareword "try_to_get_global_object" then the "r" that's passed to the get hook is the inner window.  The attached testcase shows this clearly: r === window tests false, but calling a getter on "r" that just returns "this" (which just serves to outerize) returns something that's equal to "window".
Flags: needinfo?(efaustbmo)
Assignee

Comment 1

5 years ago
My bug for sure. Recent regression partly attributable to TC39 ignoring the global/WindowProxy distinction.

I'm not sure what *should* happen here; possibly the only people who have ever considered it are cc'd on this bug...
Assignee: nobody → jorendorff
I think we should outerize just like we outerize the thisval for setters defined on the global's proto chain.
Assignee

Comment 3

5 years ago
But for all proxies? Or only scripted proxies? What about other non-native objects?
Oh, hmm....

I don't know.  :(

If this only affects proxies/non-natives on proto chains, then the only significant internal consumer we need to worry about is the global scope polluter, I suspect.  And as far as I can tell it's not actually getting a receiver passed to any of the hooks it implements, so doesn't care what happens with the receiver.  So I think we could just always outerize the receiver for proxy hooks and be ok, as long as that happens _after_ we find the right object on the proto chain where the prop lives.
Flags: sec-bounty?
Whiteboard: [reporter-external]
Looks like we are not sure of next steps here and it might be a spec bug? Critsmash team looked at rating this but are not sure of severity other than 'bad'. Anyone know how bad?
Per spec, we should be outerizing for scripted proxies (which is the only really spec-observable thing here).
bz: see comment 5
Flags: needinfo?(bzbarsky)
If the question is how bad it is that script can get its hands on an inner window, the answer is I don't know.  Bobby might...
Flags: needinfo?(bzbarsky) → needinfo?(bobbyholley)
(In reply to Boris Zbarsky [:bz] from comment #8)
> If the question is how bad it is that script can get its hands on an inner
> window, the answer is I don't know.  Bobby might...

Much less bad than it used to be. I'd guess sec-moderate.
Flags: needinfo?(bobbyholley)
Flags: sec-bounty? → sec-bounty+
Comment on attachment 8645020 [details] [diff] [review]
Outerize the receiver passed to proxy hooks

Review of attachment 8645020 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. We should think about how to keep more of these kinds of cases from biting us. Jason and I talked a little on IRC. There's not much we can do immediately better. We should try, in future, to force good test coverage with Window.

Another suggestion Jason had was to rename "inner" and "outer" windows (or at least the functions that go between them) in terms of "global" and "window proxy". To me, that does seem intuitively better, though perhaps it's hard to understandh whether you have the inner or outer one in your hand, if you're not familiar with the system.
Attachment #8645020 - Flags: review?(efaustbmo) → review+
Assignee

Comment 12

4 years ago
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/dc21224de25bd42bb8ad4728c71988dfdb26ea26
changeset:  dc21224de25bd42bb8ad4728c71988dfdb26ea26
user:       Jason Orendorff <jorendorff@mozilla.com>
date:       Fri Aug 07 10:23:05 2015 -0500
description:
Bug 1105045 - Outerize the receiver passed to proxy hooks. r=efaust.
https://hg.mozilla.org/mozilla-central/rev/dc21224de25b

Based on when this was filed, I guess this affects at least 37+. What other releases does this affect? Please nominate this for uplift as well.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(jorendorff)
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee

Comment 14

4 years ago
Comment on attachment 8645020 [details] [diff] [review]
Outerize the receiver passed to proxy hooks

Approval Request Comment
[Feature/regressing bug #]: One of the bugs under meta bug 1139700, which included a rewrite of GetProperty and SetProperty for native objects.
[User impact if declined]: inner window objects can be obtained. sec-moderate, no known exploit
[Describe test coverage new/current, TreeHerder]: the patch landed in Nightly, with tests
[Risks and why]: Not risky, as the fix is localized and straightforward. Landed in Nightly without incident.
[String/UUID change made/needed]: none
Attachment #8645020 - Flags: approval-mozilla-beta?
Attachment #8645020 - Flags: approval-mozilla-aurora?
Comment on attachment 8645020 [details] [diff] [review]
Outerize the receiver passed to proxy hooks

Security fix, taking it.
Attachment #8645020 - Flags: approval-mozilla-beta?
Attachment #8645020 - Flags: approval-mozilla-beta+
Attachment #8645020 - Flags: approval-mozilla-aurora?
Attachment #8645020 - Flags: approval-mozilla-aurora+
Backed out in https://hg.mozilla.org/releases/mozilla-aurora/rev/d8d62e273a8c because of merge conflicts. This seems to need bug 1192297 uplifted also to land cleanly.

Can we either get a branch-specific patch uploaded or get approval for 1192297 to be uplifted?
Calling esr38 wontfix since this is only sec-moderate and most of the work hanging off bug 1139700 landed on 39+ anyway.
Jason rebased and landed this.
https://hg.mozilla.org/releases/mozilla-aurora/rev/226332faa005
https://hg.mozilla.org/releases/mozilla-beta/rev/34a6fcd4793a

Jason, for the future, don't forget that pulsebot a) can't mark s-s bugs and b) doesn't mark uplifts.
Flags: needinfo?(jorendorff)
Flags: needinfo?(efaustbmo)

Updated

4 years ago
Group: core-security → core-security-release
Whiteboard: [reporter-external] → [reporter-external][post-critsmash-triage]
Whiteboard: [reporter-external][post-critsmash-triage] → [reporter-external][post-critsmash-triage][adv-main41+]
Alias: CVE-2015-4502
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.