824643 - heap-use-after-free in nsTreeBodyFrame::UpdateScrollbars

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Mats Palmgren (inactive)]

STR: load layout/xul/base/src/tree/src/409807-1.xul in an ASan build compiled
with -DDEBUG_TRACEMALLOC_PRESARENA

   871  nsTreeBodyFrame::UpdateScrollbars(const ScrollParts& aParts)
   872  {
   873    nscoord rowHeightAsPixels = nsPresContext::AppUnitsToIntCSSPixels(mRowHeight);
   874  
   875    if (aParts.mVScrollbar) {
   876      nsAutoString curPos;
   877      curPos.AppendInt(mTopRowIndex*rowHeightAsPixels);
   878      aParts.mVScrollbarContent->
   879        SetAttr(kNameSpaceID_None, nsGkAtoms::curpos, curPos, true);
   880    }
   881  
   882    if (aParts.mHScrollbar) {
   883      nsAutoString curPos;
   884      curPos.AppendInt(mHorzPosition);
   885      aParts.mHScrollbarContent->
   886        SetAttr(kNameSpaceID_None, nsGkAtoms::curpos, curPos, true);
   887    }
   888  }

The SetAttr causes the frame to be destroyed, so we'll read some poisoned
value for this->mHorzPosition on line 884.  aParts.mHScrollbarContent
is a strong ref.  It looks harmless at first glance, but filing the
bug as Security-Sensitive just in case.

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix

https://tbpl.mozilla.org/?tree=Try&rev=58eeb5c8c0d0

Comment 2

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ab11d8ed73b

Comment 3

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/rev/4ab11d8ed73b

Comment 4

[Daniel Veditz [:dveditz]]

is this a frame-poisoned DoS or should we try to get this fix on older branches as well?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I believe it's frame-poisoned.