Last Comment Bug 506871 - TreeColumns Dangling Pointer Vulnerability (ZDI-CAN-536)
: TreeColumns Dangling Pointer Vulnerability (ZDI-CAN-536)
Status: VERIFIED FIXED
[sg:critical]
: 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+2 (review requests must explain patch)
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed
.3+
.3-fixed


Attachments
XUL file as described (613 bytes, application/vnd.mozilla.xul+xml)
2009-08-02 08:22 PDT, David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
no flags Details
patch (2.66 KB, patch)
2009-08-06 18:11 PDT, David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
roc: review+
samuel.sidler+old: approval1.9.1.3+
samuel.sidler+old: approval1.9.0.14+
dveditz: approval1.8.1.next+
dbaron: approval1.8.0.next?
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 
products:

    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">
   <treecols>
     <treecol flex="1"/>
   </treecols>
   <treechildren>
   </treechildren>
</tree>

o5 = document.getElementById('tr');
o62 = o5.columns.getFirstColumn();
o5.style.MozBinding = 'url("binding.xml#bind351")';
window.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIDOMWindowUtils).redraw();
o62.columns;
o62.columns;

-- 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+2 (review requests must explain patch) 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+2 (review requests must explain patch) 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+2 (review requests must explain patch) 2009-08-06 18:11:34 PDT
Created attachment 393090 [details] [diff] [review]
patch

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+2 (review requests must explain patch) 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+2 (review requests must explain patch) 2009-08-07 14:19:00 PDT
Fixed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/ec77f4fe64a5
Comment 12 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 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+2 (review requests must explain patch) 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]
patch

Approved for 1.9.1.3 and 1.9.0.14. a=ss for release-drivers
Comment 15 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2009-08-08 20:17:08 PDT
Fixed on mozilla-1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/57be70578fac
Comment 16 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2009-08-08 20:21:30 PDT
Fix checked in to CVS trunk (for 1.9.0.14), 2009-08-08 20:20:10 -0700.
Comment 17 Al Billings [:abillings] 2009-08-18 17:52:46 PDT
Verified fixed in 1.9.0.14 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.14pre) Gecko/2009081305 GranParadiso/3.0.14pre (.NET CLR 3.5.30729). Testcase no longer crashes as it does when tested with 1.9.0.13 build.
Comment 18 Al Billings [:abillings] 2009-08-19 13:30:56 PDT
Verified fixed for 1.9.1.3 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3pre) Gecko/20090817 Shiretoko/3.5.3pre (.NET CLR 3.5.30729).
Comment 19 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 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+2 (review requests must explain patch) 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 .
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ec77f4fe64a5
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]
patch

Approved for 1.8.1.24, 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 1.8.1.24
Comment 24 Mark Banner (:standard8) (afk until 26th July) 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 1.8.1.24

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 1.8.1.24, 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: 1.7.30.4; previous revision: 1.7.30.3
done
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: 1.3.30.2; previous revision: 1.3.30.1
done

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