Closed Bug 584180 (CVE-2010-2762) Opened 10 years ago Closed 10 years ago

SJOWs create scope chains ending in outer objects


(Core :: XPConnect, defect)

Not set



Tracking Status
blocking2.0 --- betaN+
status2.0 --- ?
blocking1.9.2 --- .9+
status1.9.2 --- .9-fixed
status1.9.1 --- unaffected


(Reporter: mrbkap, Assigned: mrbkap)


(Blocks 1 open bug)


(Keywords: verified1.9.2, Whiteboard: [sg:critical?])


(3 files, 2 obsolete files)

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.
Attached patch Fix (obsolete) — Splinter Review
We'll need this on the 1.9.2 branch as well.
Assignee: nobody → mrbkap
Attachment #462513 - Flags: review?(jst)
Attached file testcase
This aborts Firefox on load.
(In reply to comment #1)
> We'll need this on the 1.9.2 branch as well.

Is 1.9.1 affected?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status1.9.2: --- → ?
status2.0: --- → ?
No, it isn't.
Attachment #462513 - Flags: review?(jst) → review+
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
blocking1.9.2: ? → .9+
blocking2.0: ? → betaN+
Whiteboard: [sg:critical?]
Attached patch Better fix (obsolete) — Splinter Review
Need to make sure we update the scope object if we're around an outer window that navigates.
Attachment #462513 - Attachment is obsolete: true
Attachment #464611 - Flags: review?(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.
Attachment #464611 - Flags: review?(jst) → review+
Attached patch With thatSplinter Review
Attachment #464611 - Attachment is obsolete: true
Attachment #465441 - Flags: review+
Attached patch For 1.9.2Splinter Review
Trivial merge.
Attachment #465443 - Flags: review+
Attachment #465443 - Flags: approval1.9.2.9?
Pushed to mozilla-central.
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 465443 [details] [diff] [review]
For 1.9.2

a=LegNeato for

This needs to be landed as soon as possible.
Attachment #465443 - Flags: approval1.9.2.9? → approval1.9.2.9+
Verified fixed in 1.9.2 using my own debug build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20100818 Namoroka/3.6.9pre ( .NET CLR 3.5.30729)).
Keywords: verified1.9.2
Alias: CVE-2010-2762
Group: core-security
You need to log in before you can comment on or make changes to this bug.