889221 - Heap-use-after-free in AssignRangeAlgorithm::implementation

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Atte Kettunen]

Attachment: Repro-file

Tested on:

OS: Ubuntu 12.04

Firefox: ASAN debug-build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-dbg-asan/1372636004/

ASAN-report:

==19409== ERROR: AddressSanitizer: heap-use-after-free on address 0x7f140911d05f at pc 0x7f14239096bf bp 0x7fffae3682d0 sp 0x7fffae3682c8
READ of size 1 at 0x7f140911d05f thread T0
    #0 0x7f14239096be in void AssignRangeAlgorithm<true, true>::implementation<unsigned char, unsigned char, unsigned int, unsigned int>(unsigned char*, unsigned int, unsigned int, unsigned char const*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray.h:570
    #1 0x7f1423909595 in unsigned char* nsTArray_Impl<unsigned char, nsTArrayInfallibleAllocator>::AppendElements<unsigned char>(unsigned char const*, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray.h:1189
    #2 0x7f1424fcb24a in mozilla::TextFrameIterator::PushBaseline(nsIFrame*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:1738
    #3 0x7f1424fcaba3 in mozilla::TextFrameIterator::Next() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:1672
    #4 0x7f1424fdb04c in nsSVGTextFrame2::UpdateFontSizeScaleFactor() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:4969
    #5 0x7f1424fda9a5 in nsSVGTextFrame2::DoReflow() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:4917
    #6 0x7f1424fcfc52 in nsSVGTextFrame2::MaybeReflowAnonymousBlockChild() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:4881
    #7 0x7f1424fd2805 in nsSVGTextFrame2::ReflowSVG() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:3549
    #8 0x7f1424f7d02b in nsSVGDisplayContainerFrame::ReflowSVG() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGContainerFrame.cpp:329
    #9 0x7f1424fb6334 in nsSVGOuterSVGFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGOuterSVGFrame.cpp:488
    #10 0x7f1423c146d0 in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/generic/nsLineLayout.cpp:830
.
.
.
freed by thread T0 here:
    #0 0x43c745 in __interceptor_realloc ??:0
    #1 0x7f142e8e75ea in moz_xrealloc /builds/slave/m-cen-l64-dbg-asan-00000000000/build/memory/mozalloc/mozalloc.cpp:86
    #2 0x7f142317312f in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyElements<unsigned char> >::EnsureCapacity(unsigned int, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/xpcom/build/../glue/nsTArray-inl.h:170
    #3 0x7f1423909579 in unsigned char* nsTArray_Impl<unsigned char, nsTArrayInfallibleAllocator>::AppendElements<unsigned char>(unsigned char const*, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray.h:1186
    #4 0x7f1424fcb24a in mozilla::TextFrameIterator::PushBaseline(nsIFrame*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:1738
    #5 0x7f1424fcaba3 in mozilla::TextFrameIterator::Next() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:1672
    #6 0x7f1424fdb04c in nsSVGTextFrame2::UpdateFontSizeScaleFactor() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:4969
    #7 0x7f1424fda9a5 in nsSVGTextFrame2::DoReflow() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:4917
    #8 0x7f1424fcfc52 in nsSVGTextFrame2::MaybeReflowAnonymousBlockChild() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:4881
    #9 0x7f1424fd2805 in nsSVGTextFrame2::ReflowSVG() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGTextFrame2.cpp:3549
    #10 0x7f1424f7d02b in nsSVGDisplayContainerFrame::ReflowSVG() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/layout/svg/nsSVGContainerFrame.cpp:329
.
.
.

Comment 1

[Cameron McCormack (:heycam)]

The

  mBaselines.AppendElement(mBaselines[mBaselines.Length() - 1]);

line isn't safe, since we pass the value in by const reference, and the underlying buffer might get reallocated before the single value gets copied in.  That seems like a bit of a footgun.

Comment 2

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 3

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

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

Review of attachment 770581 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't look like the right patch.

Comment 4

[Cameron McCormack (:heycam)]

Attachment: patch

I have too many files on my machine named a.patch.

Comment 5

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f8a786128a1

Comment 6

[Cameron McCormack (:heycam)]

In terms of impact, this bug will at worst vertically align the text incorrectly.  Unless our memory allocator overwrites the old buffer immediately after it is freed but before the value is appended (and there shouldn't be any further mallocs/frees at this point, since we are just copying a uint8_t), the correct value will be appended.

Comment 7

[Daniel Veditz [:dveditz]]

Since this is all happening on the same thread in the same instruction we're not seeing how this could be exploited in practice.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7f8a786128a1

Comment 9

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

No bounty and nothing actionable security related so this won't have an advisory.