Last Comment Bug 557174 - (CVE-2010-0183) Use-after-free error in nsCycleCollector::MarkRoots()
(CVE-2010-0183)
: Use-after-free error in nsCycleCollector::MarkRoots()
Status: RESOLVED FIXED
[sg:critical][ccbr][critsmash:resolved]
: crash, testcase, verified1.9.1
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: -- critical (vote)
: mozilla1.9.1
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 394600
  Show dependency treegraph
 
Reported: 2010-04-04 23:59 PDT by Reed Loden [:reed] (use needinfo?)
Modified: 2010-09-27 19:55 PDT (History)
11 users (show)
dveditz: wanted1.9.0.x+
jruderman: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
.10+
.10-fixed


Attachments
Testcase (crashes browser) (899 bytes, application/xml)
2010-04-05 13:24 PDT, Peter Van der Beken [:peterv] - away till Aug 1st
no flags Details
Fix (1.29 KB, patch)
2010-04-24 00:14 PDT, Boris Zbarsky [:bz]
roc: review+
christian: approval1.9.1.10+
mbeltzner: approval1.9.0.19-
Details | Diff | Splinter Review

Description Reed Loden [:reed] (use needinfo?) 2010-04-04 23:59:26 PDT
Created attachment 436992 [details]
PoC

wushi of team509 sent the following vulnerability report to security@:

================================================================================

	firefox use after free  Vulnerability 





Discovery Date:  Sep 20, 2009

Discovery By :  wushi of team509



Systems Affected



This vulnerability affects the following software :



¡¡¡¡* firefox(3.5.9 tested)

    * firefox(3.0.15 tested)



Overview



firefox  contains a vulnerability. This vulnerability may allow attackers to remotely 

execute arbitrary code on the affected system. Exploitation may occur as the result of using the 

affected webkit application to visit a website. The privileges gained by a remote attacker depend on the software 

component being attacked. 





I. Description:

	unpack the ff2.rar, copy frame.jsp and 1.xhtml files to tomcat webapp dir.use firefox to visit it(I used 

3.5.7 on windows xp sp3)  firefox will crash.



the crash will like this:



(21f8.1160): Access violation - code c0000005 (!!! second chance !!!)

eax=0073006e ebx=08d0dd00 ecx=0012bb4c edx=00000000 esi=08f902cc edi=0012bba0

eip=0073006e esp=0012bb28 ebp=08d3b5c0 iopl=0         nv up ei ng nz na po cy

cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000283

0073006e ??              ???

0:000> kv

ChildEBP RetAddr  Args to Child              

WARNING: Frame IP not in any known module. Following frames may be wrong.

0012bb24 1006dcaa 08d3b5c0 108b63f4 0012bb4c 0x73006e

0012bb88 10175670 0080b000 00000000 008d2000 xul!nsCycleCollector::MarkRoots+0x72a (FPO: [Uses EBP] [1,18,0]) (CONV: thiscall) [e:\builds\moz2_slave\win32_build\build\xpcom\base\nscyclecollector.cpp @ 1571]

0012bbec 1017f503 0080b000 00826c00 004edf9e xul!nsCycleCollector::BeginCollection+0x57 (CONV: thiscall) [e:\builds\moz2_slave\win32_build\build\xpcom\base\nscyclecollector.cpp @ 2515]

0012bbf8 004edf9e 00826c00 00000002 00826c00 xul!XPCCycleCollectGCCallback+0x3a (FPO: [2,0,0]) (CONV: cdecl) [e:\builds\moz2_slave\win32_build\build\js\src\xpconnect\src\nsxpconnect.cpp @ 390]

0012bca0 00535b5c 00826c00 00000000 00000000 js3250!js_GC+0x28e (CONV: cdecl) [e:\builds\moz2_slave\win32_build\build\js\src\jsgc.cpp @ 3504]

0012bcb4 10190ab8 00826c00 00826c00 1017f4c9 js3250!JS_GC+0x4c (FPO: [1,0,2]) (CONV: cdecl) [e:\builds\moz2_slave\win32_build\build\js\src\jsapi.cpp @ 2458]

0012bd70 101826fa 1019e122 00000001 00000002 xul!nsXPConnect::Collect+0x74 (FPO: [0,39,0]) (CONV: thiscall) [e:\builds\moz2_slave\win32_build\build\js\src\xpconnect\src\nsxpconnect.cpp @ 478]

0012fc20 101d6e5f 0080b000 00000001 101d6e29 xul!nsCycleCollector::Collect+0x8a (FPO: [2,4007,0]) (CONV: thiscall) [e:\builds\moz2_slave\win32_build\build\xpcom\base\nscyclecollector.cpp @ 2386]

0012fc2c 101d6e29 1033e3f8 101bc394 00000001 xul!nsCycleCollector_collect+0x11 (FPO: [0,0,0]) (CONV: cdecl) [e:\builds\moz2_slave\win32_build\build\xpcom\base\nscyclecollector.cpp @ 3046]

0012fc30 1033e3f8 101bc394 00000001 1019e157 xul!nsJSContext::CC+0x2a (FPO: [0,0,0]) (CONV: cdecl) [e:\builds\moz2_slave\win32_build\build\dom\src\base\nsjsenvironment.cpp @ 3534]

0012fc64 1004c1be 08d17c30 0081b5b0 0012ff40 xul!nsJSContext::MaybeCC+0x3381b7 (CONV: cdecl)







0:000> u xul!nsCycleCollector::MarkRoots+0x72a -23

xul!nsCycleCollector::MarkRoots+0x707 [e:\builds\moz2_slave\win32_build\build\xpcom\base\nscyclecollector.cpp @ 1571]:

1006dc87 3d201e0610      cmp     eax,offset xul!nsArrayCC::QueryInterface (10061e20)

1006dc8c 0f8464110000    je      xul!nsCycleCollector::MarkRoots+0x1876 (1006edf6)

1006dc92 3dc0c90910      cmp     eax,offset xul!nsEventListenerManager::QueryInterface (1009c9c0)

1006dc97 0f84ad110000    je      xul!nsCycleCollector::MarkRoots+0x18ca (1006ee4a)

1006dc9d 8d4c2414        lea     ecx,[esp+14h]

1006dca1 51              push    ecx

1006dca2 68f4638b10      push    offset xul!nsCycleCollectionISupports::COMTypeInfo<int>::kIID (108b63f4)

1006dca7 55              push    ebp

0:000> u

xul!nsCycleCollector::MarkRoots+0x728 [e:\builds\moz2_slave\win32_build\build\xpcom\base\nscyclecollector.cpp @ 1571]:

1006dca8 ffd0            call    eax



eax set value at here:



1006dc35 8b4d00          mov     ecx,dword ptr [ebp]

1006dc38 8b01            mov     eax,dword ptr [ecx]



0:000> dd ebp

08d3b5c0  01901400 08cc51b8 00000000 40300000

08d3b5d0  08debee8 00000005 00000005 00000000

08d3b5e0  08d3b600 08d3b500 08de3b60 00000000

08d3b5f0  00000000 00000000 00000000 00000000

08d3b600  08d3b620 08d3b5e0 08de3b80 00000000

08d3b610  00000000 00000000 00000000 00000000

08d3b620  08d3b640 08d3b600 08de3ba0 00000000

08d3b630  00000000 00000000 00000000 00000000

0:000> dd 01901400 

01901400  0073006e 00440049 004d004f 006f004e

01901410  00650064 00650053 0065006c 00740063

01901420  0072006f 00000000 00000000 00000000

01901430  00540053 00540041 005f0045 00530049

01901440  0044005f 0043004f 004d0055 004e0045

01901450  00000054 017e38ac 00000000 00020004

01901460  0073006e 00530049 0063006f 0065006b

01901470  00540074 00610072 0073006e 006f0070





in source code, 

void

nsCycleCollector::MarkRoots(GCGraphBuilder &builder)

{

    mGraph.mRootCount = builder.Count();



    // read the PtrInfo out of the graph that we are building

    NodePool::Enumerator queue(mGraph.mNodes);

    while (!queue.IsDone()) {

        PtrInfo *pi = queue.GetNext();

        builder.Traverse(pi);

    }

}

PRBool

nsCycleCollector::BeginCollection()

{

    if (mParams.mDoNothing)

        return PR_FALSE;



    GCGraphBuilder builder(mGraph, mRuntimes);

....................

     MarkRoots(builder);

............

}



In these functions we can know, One Graph node has been freed, so made the mistake.



in 1.xhtml, we can find the code:



<xht:isindex d=""/>



"isindex" will create a graph object, it seems because the "isindex" keyword made the mistake.











It's clear , it's a exploitable vuln.







================================================================================
Comment 1 Peter Van der Beken [:peterv] - away till Aug 1st 2010-04-05 13:24:30 PDT
Created attachment 437104 [details]
Testcase (crashes browser)

Loading this testcase and shutting down crashes 3.5 on OS X for me. In Windows the reduced testcase doesn't seem to crash. I do have the original crash recorded, trying to figure it out.
Comment 2 Damon Sicore (:damons) 2010-04-20 13:43:12 PDT
Peter, can we get an ETA here?
Comment 3 Peter Van der Beken [:peterv] - away till Aug 1st 2010-04-22 06:31:24 PDT
Something is going wrong in layout:

###!!! ASSERTION: style context has old rule node: 'n == mRuleTree', file layout/style/nsStyleSet.cpp, line 164
###!!! ASSERTION: old rule tree still referenced: 'Not Reached', file layout/style/nsStyleSet.cpp, line 924
###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file layout/base/nsPresShell.cpp, line 674

I think we end up freeing the frame, but without calling nsIsIndexFrame::Destroy first. So we never call RemoveEventListenerByIID for the frame, and the event listener manager has a pointer to a dead object in its list. At some point during CC it then tries to traverse to the dead object. On trunk the problem doesn't exist (but the listener is also separate from the frame, see nsIsIndexFrame::KeyListener).

Dbaron, bz: I tried looking into this but don't know enough about layout to debug this properly. Can one of you take over?
Comment 4 Boris Zbarsky [:bz] 2010-04-23 23:01:41 PDT
Looks like we construct the nsIsIndexFrame but it never actually ends up in the frame tree (effectively leaks).  So yes, no Destroy.
Comment 5 Boris Zbarsky [:bz] 2010-04-23 23:13:10 PDT
On trunk (and 1.9.2, since it was early enough), this got fixed in this range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=36a36a9c942b&tochange=6a5f22ccbe0e

Bug 281387 did make some frame construction changes, but they really shouldn't have affected behavior...


I'l dig into this more on Monday.
Comment 6 Boris Zbarsky [:bz] 2010-04-23 23:53:42 PDT
Bisect sez:

The first good revision is:
changeset:   30941:0a0b0c3f614b
user:        Boris Zbarsky <bzbarsky@mit.edu>
date:        Thu Jul 30 13:23:32 2009 -0400
summary:     Bug 281387.  Make nsIFrame::Append/InsertFrames use nsFrameList.  r=bernd,roc, sr=dbaron

Which is unfortunate, since we do NOT want to backport that and all its prereqs.
Comment 7 Boris Zbarsky [:bz] 2010-04-24 00:08:07 PDT
Actually, I think I see what part of that patch fixed this.  This bug was a regression from bug 394600 originally, looks like.
Comment 8 Boris Zbarsky [:bz] 2010-04-24 00:14:45 PDT
Created attachment 441248 [details] [diff] [review]
Fix

The issue was that nsMenuFrame's Append/InsertFrames methods would bail out if they found a popup.  So if the menu has two kids, a popup and something else, it would leak the something else and all its descendants.  The fix is just to take out those clearly bogus early returns after finding a popup kid.
Comment 9 Boris Zbarsky [:bz] 2010-04-26 12:58:38 PDT
Comment on attachment 441248 [details] [diff] [review]
Fix

Need branch approvals to land this, since it's not relevant on trunk or 1.9.2.
Comment 10 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-28 12:52:03 PDT
Comment on attachment 441248 [details] [diff] [review]
Fix

Too late for 1.9.1.10, we'll get this in 1.9.1.11.

Also, we're done with approval1.9.0.19, but I'll give you a=beltzner for 1.9.0 if you want to check it in.
Comment 11 Boris Zbarsky [:bz] 2010-04-28 13:03:55 PDT
I'll land on 1.9.0 if/when this gets approved for 1.9.1.
Comment 12 christian 2010-04-28 18:22:50 PDT
Comment on attachment 441248 [details] [diff] [review]
Fix

a=LegNeato for 1.9.1 (because we are waiting on a blocker and might as well get this in)
Comment 13 Reed Loden [:reed] (use needinfo?) 2010-04-28 18:46:59 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8cf59c16b1d5
Comment 14 Boris Zbarsky [:bz] 2010-04-28 20:39:17 PDT
Checking in layout/xul/base/src/nsMenuFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuFrame.cpp,v  <--  nsMenuFrame.cpp
new revision: 1.369; previous revision: 1.368
done
Comment 15 Al Billings [:abillings] 2010-05-07 11:29:06 PDT
(In reply to comment #1)
> Loading this testcase and shutting down crashes 3.5 on OS X for me. In Windows
> the reduced testcase doesn't seem to crash. I do have the original crash
> recorded, trying to figure it out.

This actually crashes in 1.9.1.9 in opt builds on shutdown on XP. It does not crash in the 1.9.1.10 builds. 

Verified for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.10) Gecko/20100504 Firefox/3.5.10 (.NET CLR 3.5.30729).
Comment 16 Jesse Ruderman 2010-09-27 19:55:37 PDT
Test: http://hg.mozilla.org/mozilla-central/rev/7ecaf6a3681e

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