Last Comment Bug 584180 - (CVE-2010-2762) SJOWs create scope chains ending in outer objects
: SJOWs create scope chains ending in outer objects
: verified1.9.2
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Andrew Overholt [:overholt]
Depends on:
Blocks: crossfuzz
  Show dependency treegraph
Reported: 2010-08-03 13:53 PDT by Blake Kaplan (:mrbkap)
Modified: 2010-10-21 23:06 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Fix (1.14 KB, patch)
2010-08-03 13:57 PDT, Blake Kaplan (:mrbkap)
jst: review+
Details | Diff | Splinter Review
testcase (82 bytes, text/html)
2010-08-03 13:59 PDT, Blake Kaplan (:mrbkap)
no flags Details
Better fix (1.56 KB, patch)
2010-08-10 15:31 PDT, Blake Kaplan (:mrbkap)
jst: review+
Details | Diff | Splinter Review
With that (1.57 KB, patch)
2010-08-12 16:12 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
Details | Diff | Splinter Review
For 1.9.2 (1.54 KB, patch)
2010-08-12 16:15 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
christian: approval1.9.2.9+
Details | Diff | Splinter Review

Description Blake Kaplan (:mrbkap) 2010-08-03 13:53:56 PDT
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) 2010-08-03 13:57:37 PDT
Created attachment 462513 [details] [diff] [review]

We'll need this on the 1.9.2 branch as well.
Comment 2 Blake Kaplan (:mrbkap) 2010-08-03 13:59:00 PDT
Created attachment 462514 [details]

This aborts Firefox on load.
Comment 3 Reed Loden [:reed] (use needinfo?) 2010-08-03 14:09:31 PDT
(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) 2010-08-03 14:11:48 PDT
No, it isn't.
Comment 5 Jesse Ruderman 2010-08-03 17:47:17 PDT
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) 2010-08-10 15:31:22 PDT
Created attachment 464611 [details] [diff] [review]
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, 2010-08-10 17:07:39 PDT
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) 2010-08-12 16:12:44 PDT
Created attachment 465441 [details] [diff] [review]
With that
Comment 9 Blake Kaplan (:mrbkap) 2010-08-12 16:15:12 PDT
Created attachment 465443 [details] [diff] [review]
For 1.9.2

Trivial merge.
Comment 10 Johnny Stenback (:jst, 2010-08-12 22:26:53 PDT
Pushed to mozilla-central.
Comment 11 christian 2010-08-12 23:45:54 PDT
Comment on attachment 465443 [details] [diff] [review]
For 1.9.2

a=LegNeato for

This needs to be landed as soon as possible.
Comment 12 Blake Kaplan (:mrbkap) 2010-08-13 13:29:33 PDT
Comment 13 Al Billings [:abillings] 2010-08-23 17:57:22 PDT
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)).

Note You need to log in before you can comment on or make changes to this bug.