Last Comment Bug 506871 - TreeColumns Dangling Pointer Vulnerability (ZDI-CAN-536)
: TreeColumns Dangling Pointer Vulnerability (ZDI-CAN-536)
: fixed1.8.1.24, regression, testcase, verified1.9.0.14, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: XP Toolkit/Widgets: XUL (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.9.2b1
Assigned To: David Baron :dbaron: ⌚️UTC-10
: Neil Deakin
Depends on:
Blocks: 221619
  Show dependency treegraph
Reported: 2009-07-28 01:10 PDT by Daniel Veditz [:dveditz]
Modified: 2010-02-05 03:26 PST (History)
11 users (show)
dbaron: blocking1.9.2+
dveditz: blocking1.9.0.14+
dveditz: wanted1.9.0.x+
dbaron: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

XUL file as described (613 bytes, application/vnd.mozilla.xul+xml)
2009-08-02 08:22 PDT, David Baron :dbaron: ⌚️UTC-10
no flags Details
patch (2.66 KB, patch)
2009-08-06 18:11 PDT, David Baron :dbaron: ⌚️UTC-10
roc: review+
samuel.sidler+old: approval1.9.1.3+
samuel.sidler+old: approval1.9.0.14+
Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2009-07-28 01:10:28 PDT
ZDI-CAN-536: Mozilla Firefox TreeColumns Dangling Pointer Vulnerability

-- ABSTRACT ------------------------------------------------------------

TippingPoint has identified a vulnerability affecting the following 

    Mozilla Firefox 3.0.x

-- VULNERABILITY DETAILS -----------------------------------------------

This vulnerability allows remote attackers to execute arbitrary code on
vulnerable installations of Mozilla Firefox. User interaction is
required to exploit this vulnerability in that the target must visit a
malicious page.

The specific flaw exists during the redrawing of tree columns contained
within a XUL document. Due to the reuse of a previously freed object,
attacker controlled memory can be executed. Successful exploitation of
this vulnerability can lead to remote compromise of the affected system
under the credentials of the currently logged in user.

When accessing the column information and then redrawing the screen, a
pointer is freed by the garbage collector, which is later accessed
allowing user controllable memory to be executed by a dangling pointer.

The following XUL document demonstrates the tree while the Javascript
accompanying it results in an exploitable condition.

<tree id="tr">
     <treecol flex="1"/>

o5 = document.getElementById('tr');
o62 = o5.columns.getFirstColumn(); = 'url("binding.xml#bind351")';

-- CREDIT --------------------------------------------------------------

This vulnerability was discovered by:
    * Anonymous
Comment 1 Reed Loden [:reed] (use needinfo?) 2009-07-28 01:17:02 PDT
Report only mentions 3.0.x. Does this not affect 3.5.x, or did they only confirm it on 3.0.x?
Comment 2 David Baron :dbaron: ⌚️UTC-10 2009-08-02 08:22:05 PDT
Created attachment 392159 [details]
XUL file as described

Doesn't crash my 3.0.10.  Are particular contents of the binding.xml file needed?
Comment 3 Reed Loden [:reed] (use needinfo?) 2009-08-02 18:21:09 PDT
(In reply to comment #2)
> Doesn't crash my 3.0.10.  Are particular contents of the binding.xml file
> needed?

dveditz followed-up with TippingPoint on that, but they have yet to respond, afaict.
Comment 7 David Baron :dbaron: ⌚️UTC-10 2009-08-06 17:49:10 PDT
Annoyingly enough, it doesn't seem to crash under valgrind, though it crashes instantly without valgrind.

Looks like it involves JS having a pointer to an nsTreeColumn whose mColumns pointer is dangling (pointing to freed memory).

It makes sense that the nsTreeColumns is gone; we've reframed the tree when we set the binding.  But the nsTreeColumns destructor should have nulled out the columns pointer back to it.  Presumably it got detached from the nsTreeColumns some other way.
Comment 8 David Baron :dbaron: ⌚️UTC-10 2009-08-06 18:11:34 PDT
Created attachment 393090 [details] [diff] [review]

Well, I looked at the code, and looked for places that might have left dangling pointers, and found two, and fixed them, and then it stopped crashing.

Plus a new assertion for bonus points.
Comment 9 David Baron :dbaron: ⌚️UTC-10 2009-08-07 06:49:56 PDT
So, for what it's worth, this seems like the sort of patch that people reading patches being landed would have a pretty good chance of figuring out how to exploit.  Any thoughts on when I should or shouldn't land it, or other ways to disguise it?
Comment 10 Daniel Veditz [:dveditz] 2009-08-07 11:15:31 PDT
People are apparently already looking at this area, so let's land ASAP
Comment 11 David Baron :dbaron: ⌚️UTC-10 2009-08-07 14:19:00 PDT
Fixed on mozilla-central:
Comment 12 David Baron :dbaron: ⌚️UTC-10 2009-08-08 09:54:13 PDT
For the record, it looks like this is a regression from bug 291531.
Comment 13 David Baron :dbaron: ⌚️UTC-10 2009-08-08 09:56:48 PDT
... well, actually, not really, since the same problem existed in a different place before that.
Comment 14 Samuel Sidler (old account; do not CC) 2009-08-08 19:38:21 PDT
Comment on attachment 393090 [details] [diff] [review]

Approved for and a=ss for release-drivers
Comment 15 David Baron :dbaron: ⌚️UTC-10 2009-08-08 20:17:08 PDT
Fixed on mozilla-1.9.1:
Comment 16 David Baron :dbaron: ⌚️UTC-10 2009-08-08 20:21:30 PDT
Fix checked in to CVS trunk (for, 2009-08-08 20:20:10 -0700.
Comment 17 Al Billings [:abillings] 2009-08-18 17:52:46 PDT
Verified fixed in with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2009081305 GranParadiso/3.0.14pre (.NET CLR 3.5.30729). Testcase no longer crashes as it does when tested with build.
Comment 18 Al Billings [:abillings] 2009-08-19 13:30:56 PDT
Verified fixed for with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20090817 Shiretoko/3.5.3pre (.NET CLR 3.5.30729).
Comment 19 David Baron :dbaron: ⌚️UTC-10 2009-09-12 14:35:16 PDT
(In reply to comment #13)
> ... well, actually, not really, since the same problem existed in a different
> place before that.

... probably introduced when the code was first written in bug 221619, but even then it could have been moved from elsewhere (though it doesn't look like it).
Comment 20 David Baron :dbaron: ⌚️UTC-10 2009-09-22 16:15:07 PDT
This landed on mozilla-central before 1.9.2 branched, but wasn't a blocker at the time we did the mass-marking, so marking status1.9.2:beta1-fixed .
Comment 21 Tracy Walker [:tracy] 2009-09-25 09:04:16 PDT
verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20090925 Namoroka/3.6b1pre
Comment 22 Daniel Veditz [:dveditz] 2010-01-19 12:42:22 PST
Comment on attachment 393090 [details] [diff] [review]

Approved for, a=dveditz for release-drivers
Comment 23 David :Bienvenu 2010-01-25 17:00:35 PST
Need to make sure this doesn't break TB 2 before landing on
Comment 24 Mark Banner (:standard8, limited time in Dec) 2010-02-05 03:26:43 PST
(In reply to comment #23)
> Need to make sure this doesn't break TB 2 before landing on

I built TB 2 and played around with various tree elements and couldn't see any issues...

(In reply to comment #22)
> (From update of attachment 393090 [details] [diff] [review])
> Approved for, a=dveditz for release-drivers

Checking in layout/xul/base/src/tree/src/nsTreeColumns.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeColumns.cpp,v  <--  nsTreeColumns.cpp
new revision:; previous revision:
Checking in layout/xul/base/src/tree/src/nsTreeColumns.h;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeColumns.h,v  <--  nsTreeColumns.h
new revision:; previous revision:

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