Last Comment Bug 555109 - (CVE-2010-1121) Move wrappers to new scope even if their parent hasn't been moved yet (ZDI-CAN-761)
: Move wrappers to new scope even if their parent hasn't been moved yet (ZDI-CA...
[sg:critical][keep hidden until 3.5.1...
: regression, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Security (show other bugs)
: unspecified
: All All
: -- critical (vote)
: mozilla1.9.3a4
Assigned To: Blake Kaplan (:mrbkap)
: David Keeler [:keeler] (use needinfo?)
Depends on: 556241
  Show dependency treegraph
Reported: 2010-03-25 17:20 PDT by Reed Loden [:reed] (use needinfo?)
Modified: 2013-03-09 10:50 PST (History)
30 users (show)
dveditz: wanted1.9.0.x+
dveditz: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

a stack (5.42 KB, text/plain)
2010-03-29 04:14 PDT, Olli Pettay [:smaug]
no flags Details
Debugging patch (5.41 KB, patch)
2010-03-29 18:33 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
wip (7.56 KB, patch)
2010-03-29 18:46 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Leak fix (4.10 KB, patch)
2010-03-30 10:10 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Splinter Review
Leak fix (7.37 KB, patch)
2010-03-30 13:08 PDT, Peter Van der Beken [:peterv]
jst: review+
Details | Diff | Splinter Review
Crash fix (7.56 KB, patch)
2010-03-30 16:16 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Crash fix (7.58 KB, patch)
2010-03-30 16:17 PDT, Blake Kaplan (:mrbkap)
jst: review+
peterv: superreview+
dveditz: approval1.9.2.3+
Details | Diff | Splinter Review
Alterna-patch sketch (9.21 KB, patch)
2010-03-30 17:50 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
For 1.9.1 (6.88 KB, patch)
2010-04-19 14:56 PDT, Blake Kaplan (:mrbkap)
jst: review+
christian: approval1.9.1.10+
Details | Diff | Splinter Review
1.8 patch (5.42 KB, patch)
2010-04-30 01:54 PDT, Martin Stránský
jst: review+
Details | Diff | Splinter Review
crash testcase (964 bytes, text/html)
2010-05-04 00:36 PDT, moz_bug_r_a4
no flags Details

Description Reed Loden [:reed] (use needinfo?) 2010-03-25 17:20:52 PDT
Placeholder for Pwn2Own bug found in Firefox at CanSecWest 2010 (CVE-2010-1121).
Comment 1 Daniel Veditz [:dveditz] 2010-03-26 13:30:39 PDT
ZDI-CAN-761: Pwn2Own: Mozilla Firefox Code Execution Vulnerability

-- ABSTRACT ------------------------------------------------------------
TippingPoint has identified a vulnerability affecting the following 

Mozilla Firefox 3.6.x

-- VULNERABILITY DETAILS -----------------------------------------------
This vulnerability was received through the 2010 Pwn2Own competition at
CanSecWest on March 24th. It is a remotely executable code execution
vulnerability affecting the latest available versions of Mozilla

The attached PoC was directly provided to us from the contestant at the
competition. Launch index.html to reproduce. It appears the crux of the
issue lies here:

function start() { 


function start2() { 
o61 = null;

location.href = 'hs.html';}

This is a cursory analysis. We expect to have a follow-up next week when
we have had time to dissect the supplied details further. We appreciate
updates and any additional details you may have on the matter as well.

Version(s) tested: Latest version of Firefox as of 3/24/2010
Platform(s) tested: Latest version of Windows 7 as of 3/24/2010

-- CREDIT --------------------------------------------------------------
This vulnerability was discovered by:

* Nils of MWR InfoSecurity
Comment 2 Daniel Veditz [:dveditz] 2010-03-26 13:32:31 PDT
Created attachment 435259 [details]
PoC from ZDI
Comment 3 chris hofmann 2010-03-26 22:06:34 PDT
> Version(s) tested: Latest version of Firefox as of 3/24/2010
> Platform(s) tested: Latest version of Windows 7 as of 3/24/2010

This article indicates the vulnerability might only be present on 64-bit system with windows 7.

Other press has indicate that the results of the test case should launch calc.exe.  We need to figure out if that is what we are watching for.

Checking the test case on a 32bit win XP vm with Firefox 3.6.2 it doesn't appear to show any problems.  The test case has been running for several hours.  It soaks up 99% cpu use and its up to 300MB of memory use, but no other indications of problems.
Comment 4 Daniel Veditz [:dveditz] 2010-03-27 09:06:11 PDT
The payload does look like the standard calc-launcher (I haven't compared to metasploit to verify). I'm not surprised that part didn't "work" if you're not on a Win7 machine, but I'm surprised it didn't crash (I don't crash on WinXp 32-bit either). Part of the exploit looks like he's trying to mimic a particular object's memory layout, maybe on the not-quite-right target it just plays nice.

Mac crashes right away though

I haven't crashed on Shiretoko on Mac, nor Minefield.
Comment 5 Daniel Veditz [:dveditz] 2010-03-27 09:11:47 PDT
Now that I've submitted the above I can crash Namoroka on 32-bit WinXP.

Comment 6 chris hofmann 2010-03-27 11:27:07 PDT
On vista I've now seen these two crashes
bp-c5e33206-3377-4343-bf0f-d73062100327 bp-1b16ab19-3b1a-4111-ad70-c0ebd2100327
and I've also now seen two cases where calc was launched.
Comment 7 chris hofmann 2010-03-27 12:00:31 PDT
no crashes for me on vista with 3.5.8 or a 3.0.x build.  they seem to pretty immune and fast and responsive when running the test case.

I've gotten a couple of crashes on 3.6b1 but no calc.exe's
these stacks look quite a bit different than the others and we crash pretty quickly on 3.6 beta1
Comment 8 chris hofmann 2010-03-27 12:19:48 PDT
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a2pre) Gecko/20090901 Namoroka/3.6a2pre  crashes but I can't get past the windows crash dialog to submit a report.
Comment 9 chris hofmann 2010-03-27 13:07:42 PDT
a rough regression range might be sometime in July

 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090630 Minefield/3.6a1pre -- no crash - response like 3.5.x

3.6a1pre  Build ID 20090731 044744  crashes like
607cbb0b-9b9b-441e-8226-afffd2100327 and 15c87ec7-d721-46dc-b8a8-339252100327
Comment 10 chris hofmann 2010-03-27 14:08:22 PDT
narrowing futher it looks like july30-july31 could be the range.  

mozilla-central/3.6a1pre builds on and before july30 behave a more like 3.5.x, and the july 31 build on windows vista crashes consistently for me within a few seconds of loading the PoC.
Comment 11 Daniel Veditz [:dveditz] 2010-03-27 17:34:18 PDT
a rough fix range on mozilla-central seems to be between 2009-11-09-04 and 1109-11-11-04. I'd check that last day in between but I'm late for dinner.
Comment 12 chris hofmann 2010-03-27 17:55:39 PDT
(In reply to comment #11)
> a rough fix range on mozilla-central seems to be between 2009-11-09-04 and
> 1109-11-11-04. I'd check that last day in between but I'm late for dinner.

NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091110 Minefield/3.7a1pre  
crashes a few seconds after loading the test case for me.
Comment 13 chris hofmann 2010-03-27 18:10:40 PDT
and no crash on Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091111 Minefield/3.7a1pre

so I think the summary is 

appears to be broken on mozilla-central after july31 and before nov10 -- then fixed then possibly fixed after that.  

3.6betas and 3.6.x don't appear to have picked up the fix after it branched.

3.5.x appears to never have been affected.
Comment 14 Daniel Veditz [:dveditz] 2010-03-27 23:05:29 PDT
Regression range (dates from comment 10):

The fix range (changesets pulled from about:buildconfig):

Nothing in the regression range stands out. I think Jonas's fix for bug 507023 was too early and would have been in the previous day's build. Maybe one of bz's checkins, like bug 281387?

None of the check-ins in the fix range seem likely to match up with damage from the bustage range.
Comment 16 Olli Pettay [:smaug] 2010-03-28 06:21:01 PDT
Verified: doesn't crash crashes

I believe something in Bug 492713 doesn't handle properly.
Maybe something in prototype handling of the old inner window, but I'm
just guessing. Need to find out the unregress-patch.
Comment 18 Olli Pettay [:smaug] 2010-03-28 07:00:23 PDT does apply to 1.9.2 
and it does fix the crash, but I wonder if that merely accidentally prevents
the crash in this case.

peter, jst, blake, you probably know this better.

Btw, I'm testing on 64bit linux.
Comment 19 Olli Pettay [:smaug] 2010-03-28 07:04:01 PDT fixed Bug 526201
(commit message misses the bug#)
Comment 20 Peter Van der Beken [:peterv] 2010-03-28 07:35:54 PDT
That's weird, it just removed a call to MorphSlimWrapper because it wasn't needed in some cases, it wasn't expected to fix any crashes. We should figure out what's going wrong without it.
Comment 21 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-28 19:40:28 PDT
So what's next steps to knowing the appropriate fix for this issue?
Comment 22 Olli Pettay [:smaug] 2010-03-29 04:14:46 PDT
Created attachment 435574 [details]
a stack

wrapper->GetFlatJSObject() seems to return either null or some
This all looks very much like a xpconnect/wrapper handling bug.
Comment 23 Olli Pettay [:smaug] 2010-03-29 04:22:58 PDT
But I'm not at all familiar with this code, so hopefully
mrbkap, peterv or jst could look at this.
Comment 24 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-29 10:33:26 PDT
Not affecting 1.9.1.x as per comment 15
Comment 25 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-29 15:42:27 PDT
Martijn: did you intend to change the blocking and status flags for the 1.9.1 branch back to ? or was that a bugzilla accident?
Comment 26 Martijn Wargers [:mwargers] (not working for Mozilla) 2010-03-29 15:53:29 PDT
Sorry, bugzilla accident.
Comment 27 Johnny Stenback (:jst, 2010-03-29 18:20:29 PDT
So this is a bug in nsXPConnect::ReparentScopeAwareWrappers() where depending on the order in which the wrappers in the given scope end up being reparented we either reparent them or we don't. The key in the testcase is the call to, which triggers the call to nsXPConnect::ReparentScopeAwareWrappers(), and if we in that code end up reparenting the document before we reparent nodes that belong to that document, we're fine, but if the order is reversed, we end up not reparenting all wrappers and later on can leave dangling wrapper pointers in XPConnect's hashes, which leads to the crash/exploit.

We have a fix that fixes this particular crash, but not all variations of it. mrbkap is working on a complete fix and expects to have that done tomorrow.

And this code goes way back, at least to 1.9.0, so 1.9.1 is for sure affected, as is trunk, the testcase just needs to be different to trigger it (but we don't know quite how).
Comment 28 Johnny Stenback (:jst, 2010-03-29 18:21:24 PDT
And kudos for finding all this out goes to mrbkap and peterv, I'm just the messenger.
Comment 29 Blake Kaplan (:mrbkap) 2010-03-29 18:33:37 PDT
Created attachment 435801 [details] [diff] [review]
Debugging patch

This patch was immensely useful to me when debugging this. I suspect it would be less useful to other people, but I'm attaching it here for reference.
Comment 30 Blake Kaplan (:mrbkap) 2010-03-29 18:46:01 PDT
Created attachment 435802 [details] [diff] [review]

This patch works, but I'm leaking a ton with it. I'll debug more tomorrow.

I also have concerns about objects that return a newParent from their PreCreate hooks that is tied to the old global. I'm considering recursively calling the PreCreate hook up the chain (and possibly memoizing results) to protect against such objects.
Comment 31 Peter Van der Beken [:peterv] 2010-03-30 07:38:29 PDT
We're leaking documents, in the CC I see a small cycle between the document for sploit/main.html , a parser and a content sink, but there's an unknown edge to the document too. I'm still trying to figure out what that edge is.
Comment 32 Peter Van der Beken [:peterv] 2010-03-30 10:10:13 PDT
Created attachment 435936 [details] [diff] [review]
Leak fix

This fixes the leaks for me. Need to hook up HTMLContentSink to CC. We probably need to add more of if members still.
Comment 33 Peter Van der Beken [:peterv] 2010-03-30 13:08:26 PDT
Created attachment 435993 [details] [diff] [review]
Leak fix

This is a more complete leak fix. I think this should be safe to do (even the unlinking). There's still the context stack, but I don't know how likely it is that we'll have cycles through that.
Can you guys confirm that this fixes all the leaks from attachment 435802 [details] [diff] [review]?
Comment 34 Johnny Stenback (:jst, 2010-03-30 13:32:24 PDT
Yes, with the last two patches applied I can leave the testcase run w/o any obvious leaks!
Comment 35 Johnny Stenback (:jst, 2010-03-30 13:54:59 PDT
Oh, and I pushed a combination of the above two patches to try.
Comment 36 Al Billings [:abillings] 2010-03-30 14:33:24 PDT
Are these trunk and 1.9.2 only?
Comment 37 Blake Kaplan (:mrbkap) 2010-03-30 14:38:55 PDT
Al, I believe that peter's patch applies to trunk (and needs massaging to work on 1.9.2)... I wrote my patch against 1.9.2, but I don't think any of the code it touches has changed in the past few years, so should apply cleanly to mozilla-central.
Comment 38 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-30 15:05:57 PDT
We should get the work landed on trunk as soon as we know it's likely to be the solution.
Comment 39 Al Billings [:abillings] 2010-03-30 15:16:13 PDT
When we check in this thing, I want us to have a good sense of what it might break and what QA can do to verify that we aren't going to ship something with unintended consequences. Right now, we have a single testcase and that doesn't seem like enough data to ship this.

Can we get automated tests for this to be checked in after we ship? Can dev give an idea of the potential consequences of this fix and where we might break Firefox or extensions, etc?
Comment 40 Blake Kaplan (:mrbkap) 2010-03-30 16:16:50 PDT
Created attachment 436052 [details] [diff] [review]
Crash fix

jst and I decided to stick with this approach. It's correct for the DOM, but potentially not if a weird extension defines certain types of C++ scriptable objects with bizarro properties.

I have a sketch of a patch (I haven't even compiled it yet) that should help deal with even weird objects, but this is far and away less risky and less code.
Comment 41 Blake Kaplan (:mrbkap) 2010-03-30 16:17:42 PDT
Created attachment 436053 [details] [diff] [review]
Crash fix

Forgot to qrefresh.
Comment 42 Johnny Stenback (:jst, 2010-03-30 17:17:59 PDT
Comment on attachment 435993 [details] [diff] [review]
Leak fix

Looks good, and tested good!
Comment 43 Blake Kaplan (:mrbkap) 2010-03-30 17:50:04 PDT
Created attachment 436089 [details] [diff] [review]
Alterna-patch sketch

This patch also fixes this bug, and is more resilient in the face of weird chains of objects that don't actually want to move. We decided that this isn't necessarily needed, so I'm just attaching it here fore reference.
Comment 44 Peter Van der Beken [:peterv] 2010-03-31 04:34:56 PDT
I've split off the leak fix into bug 556241. I'll try to make a testcase just for the leak.
Comment 45 Peter Van der Beken [:peterv] 2010-03-31 04:45:19 PDT
Comment on attachment 436053 [details] [diff] [review]
Crash fix

>diff --git a/js/src/xpconnect/idl/nsIXPConnect.idl b/js/src/xpconnect/idl/nsIXPConnect.idl

>+        JSObject *newParent = aOldScope;
>         // If the wrapper doesn't want precreate, then we don't need to
>         // worry about reparenting it.
>         if(!sciWrapper.GetFlags().WantPreCreate())
>             continue;
>-        JSObject *newParent = aOldScope;
>         rv = sciWrapper.GetCallback()->PreCreate(identity, ccx, aOldScope,
>                                                  &newParent);

That change looks unnecessary. I'll push a patch without it.
Comment 46 Peter Van der Beken [:peterv] 2010-03-31 06:57:00 PDT

Nightlies were respun and should have this fix.
Comment 47 Daniel Veditz [:dveditz] 2010-03-31 13:10:42 PDT
(In reply to comment #44)
> I've split off the leak fix into bug 556241. I'll try to make a testcase just
> for the leak.

So that bug needs to block everwhere this one does? Or we add it to the "Depends on" list? Seems like a recipe for forgetting it.

I guess for trunk that's fine, but for branches we'd like a combined patch anyway just to make sure everything goes in. (bonus points for obfuscating the actual fix a little.)
Comment 48 Daniel Veditz [:dveditz] 2010-03-31 17:38:27 PDT
Comment on attachment 436053 [details] [diff] [review]
Crash fix

Approved for and, a=dveditz for release-drivers

Please wait for the 1.9.2 branch tinderboxen to go green after checking in ( before checking into the relbranch (

Although this patch is slightly different than what was ultimately checked in, mrbkap says he will be qimporting the actual checkin to make sure the branches match.
Comment 49 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-31 19:27:17 PDT
(In reply to comment #48)
> Please wait for the 1.9.2 branch tinderboxen to go green after checking in
> ( before checking into the relbranch (

Why? We should land on both branches at the same time, IMO, so that we can spin builds as soon as the trees turn green.

mrbkap - do you think you can land these tonight? The relbranch tag is FIREFOX_3_6_2_BUILD3
Comment 50 Nick Thomas [:nthomas] 2010-03-31 19:37:04 PDT
Correction to the branch for - it's GECKO1922_20100315_RELBRANCH
Comment 51 Blake Kaplan (:mrbkap) 2010-03-31 19:59:57 PDT on the relbranch and on the 1.9.2 default branch.
Comment 52 Al Billings [:abillings] 2010-04-01 11:52:26 PDT
Verified for with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20100401 Firefox/3.6.3
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv: Gecko/20100401 Firefox/3.6.3
Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100401 Firefox/3.6.3

using the PoC, it no longer crashes.
Comment 53 Daniel Veditz [:dveditz] 2010-04-01 21:46:53 PDT
We do want a 1.9.1 branch patch for this (and bug 556241), and I'd be more comfortable having it ready just in case someone finds a way to exploit this in Firefox 3.5 and we need to firedrill.
Comment 54 christian 2010-04-15 16:17:05 PDT
Do we have an estimate of when this patch will be landed on 1.9.1? Could it possibly be done by tomorrow? Thanks!
Comment 55 Blake Kaplan (:mrbkap) 2010-04-19 14:56:30 PDT
Created attachment 440035 [details] [diff] [review]
For 1.9.1
Comment 56 christian 2010-04-20 06:15:13 PDT
Comment on attachment 440035 [details] [diff] [review]
For 1.9.1

a=LegNeato for
Comment 57 Blake Kaplan (:mrbkap) 2010-04-20 14:18:22 PDT
Comment 58 Al Billings [:abillings] 2010-04-27 12:26:38 PDT
I can't get this to crash with my 1.9.1 builds and others have noted above that the PoC, as written, doesn't trigger crashes on 1.9.1. There isn't anything that I can do to verify this for 1.9.1 specifically. The same fix in the same code fixed the issue in 1.9.2 though.
Comment 59 Martin Stránský 2010-04-30 01:54:29 PDT
Created attachment 442644 [details] [diff] [review]
1.8 patch

1.8 backport to nsIXPConnect_MOZILLA_1_8_BRANCH
Comment 60 moz_bug_r_a4 2010-05-04 00:36:50 PDT
Created attachment 443300 [details]
crash testcase

Al, does this testcase help?  Using this testcase, I can reproduce crashes on
fx-3.5.9, fx-3.6.2 and fx-3.7a4pre-2010-03-30-03.
Comment 61 Al Billings [:abillings] 2010-05-04 14:06:39 PDT
Thanks for the testcase. That crashes and passed on on XP.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20100504 Firefox/3.5.10 (.NET CLR 3.5.30729)

Marking as verified for 1.9.1.
Comment 62 Johnny Stenback (:jst, 2010-05-04 17:05:13 PDT
Comment on attachment 442644 [details] [diff] [review]
1.8 patch

Looks good, but please clean up the indentation here, remove *all* tabs.

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