890769 - Heap-buffer-overflow in gfxShapedText::CompressedGlyph::SetCanBreakBefore involving xbl:children

Status: VERIFIED FIXED

(Core :: Graphics: Text, defect)

Description

[Abhishek Arya]

Attachment: Testcase

==8944==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61a00103fd64 at pc 0x7f682e28be97 bp 0x7fff9dc04450 sp 0x7fff9dc04448
READ of size 4 at 0x61a00103fd64 thread T0
    #0 0x7f682e28be96 in gfxShapedText::CompressedGlyph::SetCanBreakBefore(unsigned char) gfx/thebes/gfxFont.h:2148
    #1 0x7f682e28b9e4 in gfxTextRun::SetPotentialLineBreaks(unsigned int, unsigned int, unsigned char*, gfxContext*) gfx/thebes/gfxFont.cpp:5407
    #2 0x7f681ffe373d in BuildTextRunsScanner::BreakSink::SetBreaks(unsigned int, unsigned int, unsigned char*) layout/generic/nsTextFrameThebes.cpp:914
    #3 0x7f68216fbb15 in nsLineBreaker::AppendText(nsIAtom*, unsigned char const*, unsigned int, unsigned int, nsILineBreakSink*) content/base/src/nsLineBreaker.cpp:445
    #4 0x7f681fefd00b in BuildTextRunsScanner::SetupBreakSinksForTextRun(gfxTextRun*, void const*, unsigned int) layout/generic/nsTextFrameThebes.cpp:2305
    #5 0x7f681fee8f5e in BuildTextRunsScanner::SetupLineBreakerContext(gfxTextRun*) layout/generic/nsTextFrameThebes.cpp:2210
    #6 0x7f681fee57db in BuildTextRunsScanner::FlushFrames(bool, bool) layout/generic/nsTextFrameThebes.cpp:1392
    #7 0x7f681ff07acf in BuildTextRuns(gfxContext*, nsTextFrame*, nsIFrame*, nsLineList_iterator const*, nsTextFrame::TextRunType) layout/generic/nsTextFrameThebes.cpp:1340
    #8 0x7f681ff031e6 in nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, gfxContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) layout/generic/nsTextFrameThebes.cpp:2465
    #9 0x7f681ff806c7 in nsTextFrame::ReflowText(nsLineLayout&, int, nsRenderingContext*, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsTextFrameThebes.cpp:7664
    #10 0x7f681fda8ced in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:837
    #11 0x7f681fa46ac9 in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:3696
    #12 0x7f681fa41784 in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3493
    #13 0x7f681fa35852 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3351
    #14 0x7f681fa26162 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:2492
    #15 0x7f681fa0fdbd in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2011
    #16 0x7f681fa0350c in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1010
    #17 0x7f681faf0629 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #18 0x7f681fac2047 in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsCanvasFrame.cpp:487
    #19 0x7f681faf0629 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #20 0x7f681fc51201 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:445
    #21 0x7f681fc55380 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) layout/generic/nsGfxScrollFrame.cpp:545
    #22 0x7f681fc59845 in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsGfxScrollFrame.cpp:786
    #23 0x7f681faf0629 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #24 0x7f682003e14e in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsViewportFrame.cpp:225
    #25 0x7f681f774cf2 in PresShell::DoReflow(nsIFrame*, bool) layout/base/nsPresShell.cpp:7845
    #26 0x7f681f7a21a1 in PresShell::ProcessReflowCommands(bool) layout/base/nsPresShell.cpp:7986
    #27 0x7f681f7a095d in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) layout/base/nsPresShell.cpp:3897
    #28 0x7f681f862052 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:1183
    #29 0x7f681f89347a in mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver*, long, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:171
    #30 0x7f681f89298e in mozilla::RefreshDriverTimer::Tick() layout/base/nsRefreshDriver.cpp:163
    #31 0x7f681f891e9a in mozilla::RefreshDriverTimer::TimerTick(nsITimer*, void*) layout/base/nsRefreshDriver.cpp:188
    #32 0x7f682db6a0b2 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:543
    #33 0x7f682db6b46a in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:627
    #34 0x7f682db35677 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:626
    #35 0x7f682d79f9d2 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
    #36 0x7f6827a1324b in nsXULWindow::ShowModal() xpfe/appshell/src/nsXULWindow.cpp:364
    #37 0x7f68279bc197 in nsContentTreeOwner::ShowAsModal() xpfe/appshell/src/nsContentTreeOwner.cpp:523
    #38 0x7f68279bc348 in non-virtual thunk to nsContentTreeOwner::ShowAsModal() xpfe/appshell/src/nsContentTreeOwner.cpp:524
    #39 0x7f68277a152c in nsWindowWatcher::OpenWindowInternal(nsIDOMWindow*, char const*, char const*, char const*, bool, bool, bool, nsIArray*, nsIDOMWindow**) embedding/components/windowwatcher/src/nsWindowWatcher.cpp:1005
    #40 0x7f6827795884 in nsWindowWatcher::OpenWindow(nsIDOMWindow*, char const*, char const*, char const*, nsISupports*, nsIDOMWindow**) embedding/components/windowwatcher/src/nsWindowWatcher.cpp:344
    #41 0x7f682dc5f0bb in NS_InvokeByIndex xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
    #42 0x7f6827073ad0 in CallMethodHelper::Invoke() js/xpconnect/src/XPCWrappedNative.cpp:2804
    #43 0x7f6827073ad0 in CallMethodHelper::Call() js/xpconnect/src/XPCWrappedNative.cpp:2142
    #44 0x7f6827073ad0 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) js/xpconnect/src/XPCWrappedNative.cpp:2108
    #45 0x7f68270cc0df in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1315
    #46 0x7f68355bb60c in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:321
    #47 0x7f68355bb60c in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:479
    #48 0x7f683559a7c5 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2294
    #49 0x7f683554911e in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:436
    #50 0x7f68355bbc94 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:498
    #51 0x7f68355bfb53 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:529
    #52 0x7f6835e96784 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5729
    #53 0x7f6827033030 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1437
    #54 0x7f6826ffde3b in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:589
    #55 0x7f682dc64764 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
    #56 0x7f682dc6181a in SharedStub
0x61a00103fd64 is located 0 bytes to the right of 356-byte region [0x61a00103fc00,0x61a00103fd64)
allocated by thread T0 here:
    #0 0x41a6d2 in malloc
    #1 0x7f683ea6faee in moz_malloc memory/mozalloc/mozalloc.cpp:64
    #2 0x7f682e289381 in gfxTextRun::AllocateStorageForTextRun(unsigned long, unsigned int) gfx/thebes/gfxFont.cpp:5312
    #3 0x7f682e26cb50 in gfxTextRun::Create(gfxTextRunFactory::Parameters const*, unsigned int, gfxFontGroup*, unsigned int) gfx/thebes/gfxFont.cpp:5329
    #4 0x7f682e26ffd4 in gfxFontGroup::MakeTextRun(unsigned char const*, unsigned int, gfxTextRunFactory::Parameters const*, unsigned int) gfx/thebes/gfxFont.cpp:4310
    #5 0x7f681fefabe5 in gfxTextRun* MakeTextRun<unsigned char>(unsigned char const*, unsigned int, gfxFontGroup*, gfxTextRunFactory::Parameters const*, unsigned int) layout/generic/nsTextFrameThebes.cpp:559
    #6 0x7f681fef01e0 in BuildTextRunsScanner::BuildTextRunForFrames(void*) layout/generic/nsTextFrameThebes.cpp:2051
    #7 0x7f681fee5cd9 in BuildTextRunsScanner::FlushFrames(bool, bool) layout/generic/nsTextFrameThebes.cpp:1411
    #8 0x7f681ff07acf in BuildTextRuns(gfxContext*, nsTextFrame*, nsIFrame*, nsLineList_iterator const*, nsTextFrame::TextRunType) layout/generic/nsTextFrameThebes.cpp:1340
    #9 0x7f681ff031e6 in nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, gfxContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) layout/generic/nsTextFrameThebes.cpp:2465
    #10 0x7f681ff806c7 in nsTextFrame::ReflowText(nsLineLayout&, int, nsRenderingContext*, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsTextFrameThebes.cpp:7664
    #11 0x7f681fda8ced in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:837
    #12 0x7f681fa46ac9 in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:3696
    #13 0x7f681fa41784 in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3493
    #14 0x7f681fa35852 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3351
    #15 0x7f681fa26162 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:2492
    #16 0x7f681fa0fdbd in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2011
    #17 0x7f681fa0350c in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1010
    #18 0x7f681faf0629 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #19 0x7f681fac2047 in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsCanvasFrame.cpp:487
    #20 0x7f681faf0629 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #21 0x7f681fc51201 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:445
    #22 0x7f681fc55380 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) layout/generic/nsGfxScrollFrame.cpp:545
    #23 0x7f681fc59845 in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsGfxScrollFrame.cpp:786
    #24 0x7f681faf0629 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #25 0x7f682003e14e in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsViewportFrame.cpp:225
    #26 0x7f681f774cf2 in PresShell::DoReflow(nsIFrame*, bool) layout/base/nsPresShell.cpp:7845
    #27 0x7f681f7a21a1 in PresShell::ProcessReflowCommands(bool) layout/base/nsPresShell.cpp:7986
    #28 0x7f681f7a095d in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) layout/base/nsPresShell.cpp:3897
    #29 0x7f681f79efb7 in PresShell::FlushPendingNotifications(mozFlushType) layout/base/nsPresShell.cpp:3743
Shadow bytes around the buggy address:
  0x0c34801fff50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801fff60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801fff70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801fff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c34801fff90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c34801fffa0: 00 00 00 00 00 00 00 00 00 00 00 00[04]fa fa fa
  0x0c34801fffb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801fffc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801fffd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801fffe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c34801ffff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:     fa
  Heap righ redzone:     fb
  Freed Heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==8944==ABORTING

Comment 1

[Jonathan Kew [:jfkthame]]

Like the testcase in bug 888728, this one also involves creating <xbl:children>, and in a debug build, it throws assertions beginning with the same "Ancestors of nodes..." one. I suspect it may have the same underlying cause...

###!!! ASSERTION: Ancestors of nodes with frames to be constructed lazily should have frames: '!noPrimaryFrame', file /Users/jkew/mozdev/mc/layout/base/nsCSSFrameConstructor.cpp, line 6249
###!!! ASSERTION: Ancestors of nodes with frames to be constructed lazily should not have NEEDS_FRAME bit set: '!needsFrameBitSet', file /Users/jkew/mozdev/mc/layout/base/nsCSSFrameConstructor.cpp, line 6251
###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file /Users/jkew/mozdev/mc/gfx/thebes/gfxSkipChars.cpp, line 61
###!!! ASSERTION: Overflow: 'aStart + aLength <= GetLength()', file /Users/jkew/mozdev/mc/gfx/thebes/gfxFont.cpp, line 5394

Comment 2

[Daniel Veditz [:dveditz]]

What kind of object is being read here and what could go wrong with it. 4 bytes doesn't sound like a pointer, but isn't a character either.

Comment 3

[Jonathan Kew [:jfkthame]]

The read operation isn't important/dangerous in itself, but the next thing that would happen - if ASAN hadn't killed the process - would be a write to the same location.

gfxShapedText::CompressedGlyph::SetCanBreakBefore() is trying to set a two-bit flags field within the 32-bit CompressedGlyph record corresponding to a given character offset in the textrun. The CompressedGlyph record is stored as a uint32_t and manipulated with bitwise operators, so it makes sense that to set the BreakBefore bits, the code will first read the 32-bit value, then apply some bit operations and store the new value.

So AFAICT, SetCanBreakBefore is modifying two bits that it shouldn't be touching - the length of the textrun does not match the length of the text that it was expected to cover, and so instead of a CompressedGlyph record in the textrun, we're touching whatever object immediately follows the textrun in memory.

Presumably we might proceed to touch the same two-bit field within successive 32-bit words, if the length mismatch is more than a single character. But the fact that this code will -only- modify (at most) two bits within each 4-byte "record" may help to limit how much mischief could be accomplished here.

I suspect the underlying problem here (and in bug 888728) may be that we're getting into some kind of state where we have "stale" textruns whose length no longer matches the length of the content text that we thought they matched.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

This should have been fixed by the patch in bug 890775 -- Abhishek, can you confirm with a nightly build (the first nightly with the patch will be tomorrow's)?

Comment 5

[Abhishek Arya]

(In reply to Blake Kaplan (:mrbkap) from comment #4)
> This should have been fixed by the patch in bug 890775 -- Abhishek, can you
> confirm with a nightly build (the first nightly with the patch will be
> tomorrow's)?

Sure. Can you cc me on 890775. it becomes easier to check if the patch made it to m-c.

Comment 6

[Abhishek Arya]

Blake, I checked on trunk, this testcase and the testcase in bug 888728 don't reproduce anymore.

Comment 9

[Daniel Veditz [:dveditz]]

Duplicating the older bug 888728 to this one rather than vice versa because it's clearer with this testcase that this is a sec-critical problem. The patch in bug 890775 fixes the underlying issue for both.

Comment 10

[Matt Wobensmith [:mwobensmith][:matt:]]

Confirmed crash in FF25, 2013-07-11.
Verified fixed in FF25, 2013-10-01.