Last Comment Bug 484320 - (CVE-2009-1044) XUL <tree> _moveToEdgeShift garbage-collection exploit (zdi-can-465)
: XUL <tree> _moveToEdgeShift garbage-collection exploit (zdi-can-465)
[sg:critical] ZDI CanSecWest 2009 bug...
: crash, fixed1.8.1.22, verified1.9.0.8, verified1.9.0.9, verified1.9.1
Product: Core
Classification: Components
Component: XUL (show other bugs)
: unspecified
: All All
: P1 critical (vote)
: ---
Assigned To: Olli Pettay [:smaug]
: Neil Deakin
Depends on:
  Show dependency treegraph
Reported: 2009-03-19 19:24 PDT by Daniel Veditz [:dveditz]
Modified: 2010-03-25 17:18 PDT (History)
22 users (show)
mbeltzner: blocking1.9.1+
dveditz: blocking1.9.0.8+
dveditz: blocking1.9.0.9+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (716 bytes, patch)
2009-03-23 15:38 PDT, Olli Pettay [:smaug]
dveditz: review+
mrbkap: superreview+
dveditz: approval1.9.0.9+
Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2009-03-19 19:24:50 PDT
Placeholder for Pwn2Own bug found in Firefox at CanSecWest 2009
Comment 3 Daniel Veditz [:dveditz] 2009-03-23 14:17:16 PDT
jst said to start with Neil. Since this is a high profile bug (Firefox cracked during a public hacking contest) we need to focus on it. If we had a fix I'd like to shoehorn it into even though we're past codefreeze (April release) but May's is more realistic. Needs to make 3.5b4.
Comment 4 Olli Pettay [:smaug] 2009-03-23 14:44:31 PDT
Windows and OSX stacks look quite different :/

Does this crash on trunk/1.9.1?
If not, my guess is that bug 430214 fixed this.
Especially this
It changed nsTreeSelection::FireOnSelectHandler to asynchronous.
Comment 5 Olli Pettay [:smaug] 2009-03-23 14:49:23 PDT
(In reply to comment #4)
> Does this crash on trunk/1.9.1?
I was told it does crash, so something else then...
Comment 6 Daniel Veditz [:dveditz] 2009-03-23 14:57:44 PDT
Here's yet a different Windows crash, in Shiretoko rather than 3.0.7

what does nsTextServicesDocument::InitWithEditor have to do with this testcase?
Comment 7 Blake Kaplan (:mrbkap) 2009-03-23 15:00:24 PDT
This sounds like memory stomping somewhere. valgrind might shed some light.
Comment 8 Daniel Veditz [:dveditz] 2009-03-23 15:07:48 PDT
Created attachment 368965 [details]
safer starting point

Here's a safer starting point. The guts of the testcase are still in the, but this copy of hold.html has the shellcode replaced with %u4141...
Comment 9 Mike Beltzner [:beltzner, not reading bugmail] 2009-03-23 15:25:19 PDT
Yeah, need to fix this for the reasons stated above. Neil: are you the right dude to be looking at this?
Comment 10 Olli Pettay [:smaug] 2009-03-23 15:38:39 PDT
Created attachment 368977 [details] [diff] [review]

Could someone please test this on non-linux.
Comment 11 Olli Pettay [:smaug] 2009-03-23 15:39:22 PDT
I haven't yet checked if timer code should be actually fixed.
Comment 12 Olli Pettay [:smaug] 2009-03-23 15:40:18 PDT
I tested this on 1.9.1 / 64bit linux / debug build
Comment 13 Olli Pettay [:smaug] 2009-03-23 15:42:53 PDT
(In reply to comment #11)
> I haven't yet checked if timer code should be actually fixed.
But for 1.9.0 the patch might be the safest fix - shouldn't cause regressions.
Comment 14 Daniel Veditz [:dveditz] 2009-03-23 15:45:13 PDT
Note that I never got the PoC to crash in a debug build so the patch will have to be verified in an opt build.
Comment 15 Olli Pettay [:smaug] 2009-03-23 15:48:02 PDT
Comment on attachment 368977 [details] [diff] [review]

So, InitWithFuncCallback is used, passing this is unsafe *unless*
timer is canceled before deleting this.
Comment 16 Blake Kaplan (:mrbkap) 2009-03-23 16:46:42 PDT
Comment on attachment 368977 [details] [diff] [review]

This makes sense. Good catch!
Comment 17 Daniel Veditz [:dveditz] 2009-03-23 17:35:07 PDT
Comment on attachment 368977 [details] [diff] [review]


Tested in opt builds and this patch stops the mac crashes I was seeing (I never got a debug build to crash). Since I was seeing completely different stacks on windows I'd like to verify this there as well.
Comment 18 Mike Beltzner [:beltzner, not reading bugmail] 2009-03-23 22:40:24 PDT
Comment on attachment 368977 [details] [diff] [review]

Does this have to go in on trunk and branch at the same time, or can we bake it there a bit first? Either way, it's a blocker, so doesn't need a191.
Comment 19 Olli Pettay [:smaug] 2009-03-24 08:46:08 PDT

This does still need verification on windows.
Comment 20 Olli Pettay [:smaug] 2009-03-24 09:10:04 PDT
The patch does fix the crash also on 1.9.0 (tested on linux).
Comment 21 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-03-24 09:14:57 PDT
The patch seems to fix the crash for me in my trunk debug build on windows XP.
Comment 22 Daniel Veditz [:dveditz] 2009-03-24 10:11:28 PDT
Comment on attachment 368977 [details] [diff] [review]

Approved for, a=dveditz for release-drivers

Can you land this ASAP? we're well beyond code-freeze for
Comment 23 Olli Pettay [:smaug] 2009-03-24 10:17:10 PDT
Checking in layout/xul/base/src/tree/src/nsTreeSelection.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp,v  <--  nsTreeSelection.cpp
new revision: 1.59; previous revision: 1.58
Comment 24 Daniel Veditz [:dveditz] 2009-03-24 19:20:15 PDT
Stops the crash on windows for me as well (tested shiretoko tinderbox build).
Comment 25 Samuel Sidler (old account; do not CC) 2009-03-25 15:11:15 PDT
Olli: Does this bug affect 1.8? We're taking it in a firedrilled Firefox 3.0.8, so if so, we'll want to get a patch ready for the distros to take.
Comment 26 Olli Pettay [:smaug] 2009-03-25 15:24:33 PDT
I can't reproduce the crash on 1.8.1, but I don't know why.
The code is the same.

The patch applies to 1.8.1 too (just need to use --fuzz=3)
Comment 27 Olli Pettay [:smaug] 2009-03-25 15:30:44 PDT
Ah, 1.8.1 doesn't seem to have bug 362680. So reproducing would need some
changes to the testcase.
Comment 28 Christopher Aillon (sabbatical, not receiving bugmail) 2009-03-25 16:53:31 PDT
1.8.0 is in the same boat as above.  patch applies with fuzz=3, but testcase doesn't work
Comment 29 Johnathan Nightingale [:johnath] 2009-03-26 07:47:36 PDT
Should this have a crashtest?
Comment 30 Olli Pettay [:smaug] 2009-03-26 07:54:44 PDT
Not yet, IMO. As far as I know the testcase is not public yet.
Comment 31 Josh Bressers 2009-03-26 08:21:29 PDT
This is CVE-2009-1044
Comment 32 Olli Pettay [:smaug] 2009-03-26 08:46:37 PDT
(In reply to comment #31)
> This is CVE-2009-1044
But the testcase isn't public, right?
Comment 33 Josh Bressers 2009-03-26 09:31:26 PDT
No, MITRE assigned this based only off of the announcement from CanSecWest.  If you look at the CVE id information:

There is nothing of consequence there.  It's just a placeholder.
Comment 34 juan becerra [:juanb] 2009-03-26 19:01:49 PDT
Verified using: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/2009032608 Firefox/3.0.8

Also verified with Fx308 on Ubuntu 8.10 and Windows XP and Vista.

Mac Fx307 crashed with the test case.

Fx307 did not crash on Linux nor Windows(xp/vista) VMs for me. Fx307 did crash on a windows installation running on hardware, however.
Comment 35 Daniel Veditz [:dveditz] 2009-04-01 16:40:07 PDT
Although the exploit doesn't affect the 1.8 branch because it uses functionality that doesn't exist there, we should take this small patch just in case there's another way to end up with a dangling selection timer.
Comment 36 Al Billings [:abillings] 2009-04-02 15:39:22 PDT
Verified for with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2009040206 GranParadiso/3.0.9pre (.NET CLR 3.5.30729) and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/2009040204 GranParadiso/3.0.9pre.

Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090402 Shiretoko/3.5b4pre.
Comment 37 Alexander Sack 2009-04-28 04:31:41 PDT
Comment on attachment 368977 [details] [diff] [review]

code is the same, so it seems it makes sense to take this.
Comment 38 Alexander Sack 2009-04-28 04:32:54 PDT
Comment on attachment 368977 [details] [diff] [review]

a=asac for 1.8.0
Comment 39 Daniel Veditz [:dveditz] 2009-05-29 23:44:25 PDT
fix checked into the 1.8.1 branch
Checking in layout/xul/base/src/tree/src/nsTreeSelection.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp,v  <--  nsTreeSelection.cpp
new revision:; previous revision: 1.47

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