506871 - TreeColumns Dangling Pointer Vulnerability (ZDI-CAN-536)

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Daniel Veditz [:dveditz]]

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]]

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-4, no longer working on Mozilla)]

Attachment: 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]]

(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-4, no longer working on Mozilla)]

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-4, no longer working on Mozilla)]

Attachment: 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-4, no longer working on Mozilla)]

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]]

People are apparently already looking at this area, so let's land ASAP

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/ec77f4fe64a5

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

For the record, it looks like this is a regression from bug 291531.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

... well, actually, not really, since the same problem existed in a different place before that.

Comment 14

[Samuel Sidler (old account; do not CC)]

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-4, no longer working on Mozilla)]

Fixed on mozilla-1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/57be70578fac

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to CVS trunk (for 1.9.0.14), 2009-08-08 20:20:10 -0700.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

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 - ex-MoCo]]

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-4, no longer working on Mozilla)]

(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-4, no longer working on Mozilla)]

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]]

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]]

Comment on attachment 393090 [details] [diff] [review]
patch

Approved for 1.8.1.24, a=dveditz for release-drivers

Comment 23

[David :Bienvenu]

Need to make sure this doesn't break TB 2 before landing on 1.8.1.24

Comment 24

[Mark Banner (:standard8)]

(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

Comment 25

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336