Bugzilla@Mozilla – Bug 571106
nsTreeSelection Dangling Pointer Remote Code Execution Vulnerability (ZDI-CAN-755)
Last modified: 2010-09-27 18:05:55 PDT
Summon comment box
Created attachment 450220 [details] XUL testcase ZDI-CAN-755: Mozilla Firefox nsTreeSelection Dangling Pointer 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 implementation of XUL <tree> element's "selection" attribute. There is an integer overflow when calculating the bounds of a new selection range. When calling adjustSelection on this manged range both ranges are deleted leaving a dangling reference. A remote attacker can exploit this vulnerability to execute arbitrary code under the context of the browser. View of XUL <tree> element exposes "selection" attribute. This in turn allows user to precisely choose set of tree's rows to be shown as selected. The way class nsTreeSelection (layout/xul/base/src/tree/src/nsTreeSelection.cpp) is implemented is quite interesting: with a linked list of nsTreeRange instances where nsTreeRange represents single continuous range of selected rows (and possibly a pointer to the next range). nsTreeSelection holds directly only one pointer, to the first range, mFirstRange. What is also important to note: whenever any nsTreeRange instance is deleted, delete (inside destructor) is called recursively on the next linked instances as well. Imagine we have the following piece of Javascript code: [1] sel = tree.treeBoxObject.view.selection; [2] sel.rangedSelect(0, 0x7fffffff, true); [3] sel.adjustSelection(1, 1); At [1] we grab the reference to selection object. At this moment mFirstRange is null, no row is selected, so there is no need for any range to keep track of. Then [2] we call method rangedSelect(). From nsTreeSelection::RangedSelect(): ... PRInt32 start = aStartIndex < aEndIndex ? aStartIndex : aEndIndex; PRInt32 end = aStartIndex < aEndIndex ? aEndIndex : aStartIndex; ... nsTreeRange* range = new nsTreeRange(this, start, end); if (!range) return NS_ERROR_OUT_OF_MEMORY; range->Invalidate(); if (aAugment && mFirstRange) mFirstRange->Insert(range); else mFirstRange = range; ... That makes mFirstRange being set to range <0, 2G>, or more precisely (yet in ad hoc pseudo code): mFirstRange = { min: 0, max: 2G, prev: null, next: null } And at [3] we hit integer overflow that causes a lot of trouble. From nsTreeSelection::AdjustSelection(PRInt32 aIndex, PRInt32 aCount): ... nsTreeRange* newRange = nsnull; PRBool selChanged = PR_FALSE; nsTreeRange* curr = mFirstRange; while (curr) { if (aCount > 0) { // inserting if (aIndex > curr->mMax) { ... } else if (aIndex <= curr->mMin) { ... } else { // adjustment happen inside the range. // break apart the range and create two ranges ADD_NEW_RANGE(newRange, this, curr->mMin, aIndex - 1); ADD_NEW_RANGE(newRange, this, aIndex + aCount, curr->mMax + aCount); selChanged = PR_TRUE; } } else { ... } curr = curr->mNext; } delete mFirstRange; mFirstRange = newRange; ... As we are calling AdjustSelection(1, 1), two ADD_NEW_RANGE() are executed. This means that something like <0, 0>.Insert(<2, -2G>) will be called. Notice that because of integer overflow lower bound (2) is above higher bound (-2G). void Insert(nsTreeRange* aRange) { if (mMin >= aRange->mMax) aRange->Connect(mPrev, this); else if (mNext) mNext->Insert(aRange); else aRange->Connect(this, nsnull); } So, now <2, -2G>.Connect(mPrev = null, <0, 0>): void Connect(nsTreeRange* aPrev = nsnull, nsTreeRange* aNext = nsnull) { if (aPrev) aPrev->mNext = this; else mSelection->mFirstRange = this; if (aNext) aNext->mPrev = this; mPrev = aPrev; mNext = aNext; } Which results in: mFirstRange = { min: 2, max: -2G, prev: null, next: { min: 0, max: 0, prev: mFirstRange, next: null } } But we can not forget about the epilogue of AdjustSelection(): delete mFirstRange; mFirstRange = newRange; Final result: both ranges (<2, -2G>, <0, 0>) are deleted and mFirstRange points at deleted range <0, 0>. -- CREDIT -------------------------------------------------------------- This vulnerability was discovered by: * regenrecht
bp-99ea876b-1fe9-4df4-924b-dfdde2100609
On Mac 3.5.11pre and 3.6.6pre it looks like a null-deref DoS 3.5: bp-2d7a4637-6ce5-47b0-867c-a41ca2100609 3.6: bp-c584be08-5186-402e-bee2-f6af42100609 I get a similar Mac trunk crash to Reed's Linux one above. bp-e901aaaf-6b71-4603-8d96-64f722100609 The code looks the same though, so I'd expect all versions to have the same issues as described in comment 0.
Created attachment 450359 [details] [review] simple version Make sure nsTreeRange is created in a proper way. Other cases when nsTreeRange is created manually without the macro are safe.
Comment on attachment 450359 [details] [review] simple version >+ PRInt32 start = macro_start; \ Not sure why you created a temporary for macro_start but still double-evaluated macro_end? >+ PRInt32 end = start < macro_end ? macro_end : start; \ Nit: start > macro_end is the (hopefully unlikely) bug case; start = macro_end and start < macro_end are equally likely, so this hurts branch prediction.
ok, I'll tweak the patch a bit.
I'll update the patch tomorrow.
Created attachment 451552 [details] [review] patch
http://hg.mozilla.org/mozilla-central/rev/1353f853b17a
Comment on attachment 451552 [details] [review] patch Approved for 1.9.2.6 and 1.9.1.11, a=dveditz for release-drivers
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/9003d01b2009 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a995876d5219
Verified for 1.9.1.11 using attached testcase and build 1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.11) Gecko/20100701 Firefox/3.5.11 (.NET CLR 3.5.30729)). Verified for 1.9.2.7 with its build 1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7) Gecko/20100701 Firefox/3.6.7 (.NET CLR 3.5.30729)). Verified that 1.9.1.10 and 1.9.2.6 both crash with the testcase.
Comment on attachment 451552 [details] [review] patch Requesting approval1.9.0.next on this patch so that we can take it in upcoming Camino 2.0.x security and stability updates. If approved, I'll handle the checkins, unless the patch author requests otherwise.
Comment on attachment 451552 [details] [review] patch Approved for 1.9.0.20, a=dveditz
Checking in layout/xul/base/src/tree/src/nsTreeSelection.cpp; /cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp,v <-- nsTreeSelection.cpp new revision: 1.63; previous revision: 1.62 done
We need an in-tree regression test for this.
making private again while we fix bug 585815