584180 - (CVE-2010-2762) SJOWs create scope chains ending in outer objects

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

A SJOW around an outer window creates a scope function parented directly to the outer window's global object, which is... the outer window.

I don't know if this is exploitable, but it's definitely hitting us on bug 581539, because window.postMessage indirectly depends on the scope chain ending in an inner object.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

We'll need this on the 1.9.2 branch as well.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: testcase

This aborts Firefox on load.

Comment 3

[Reed Loden [:reed]]

(In reply to comment #1)
> We'll need this on the 1.9.2 branch as well.

Is 1.9.1 affected?

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

No, it isn't.

Comment 5

[Jesse Ruderman]

Loading the testcase in an opt build doesn't do anything obviously bad, but in a debug build I get:

###!!! ABORT: should have gotten an inner window here: 'callerInnerWin->IsInnerWindow()', file /Users/jruderman/mozilla-central/dom/base/nsGlobalWindow.cpp, line 5442

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Better fix

Need to make sure we update the scope object if we're around an outer window that navigates.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 464611 [details] [diff] [review]
Better fix

+  if (JSVAL_IS_OBJECT(v)) {
+    JSObject *funobj = JSVAL_TO_OBJECT(v);
+    if (JS_GetGlobalForObject(cx, funobj) != scopeobj &&
+        !JS_SetParent(cx, funobj, scopeobj)) {
+      return nsnull;
+    }
+
+    return funobj;

I think I'd rather see us create a new function in this case than to change the existing function's scope in this rare case, just in case there's a way to exploit this function whose scope ends up changing through some bizarre way.

r=jst with that.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: With that

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: For 1.9.2

Trivial merge.

Comment 10

[Johnny Stenback (:jst)]

Pushed to mozilla-central.

http://hg.mozilla.org/mozilla-central/rev/11ca949a6aff

Comment 11

[LegNeato]

Comment on attachment 465443 [details] [diff] [review]
For 1.9.2

a=LegNeato for 1.9.2.9.

This needs to be landed as soon as possible.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/29fac3124689

Comment 13

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

Verified fixed in 1.9.2 using my own debug build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9pre) Gecko/20100818 Namoroka/3.6.9pre ( .NET CLR 3.5.30729)).