888728 - Out of bounds read-2 in BuildTextRunsScanner::ScanFrame

Status: VERIFIED DUPLICATE of bug 890769

(Core :: Layout: Text and Fonts, defect)

Description

[Abhishek Arya]

Attachment: Testcase

==1606==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61902a674520 at pc 0x7fae49a8bfcf bp 0x7fff09edaa50 sp 0x7fff09edaa48
READ of size 2 at 0x61902a674520 thread T0
    #0 0x7fae49a8bfce in nsTextFragment::CharAt(int) const content/base/src/nsTextFragment.h:163
    #1 0x7fae4a673f06 in HasTerminalNewline(nsTextFrame const*) layout/generic/nsTextFrameThebes.cpp:1495
    #2 0x7fae4a6749c7 in BuildTextRunsScanner::ScanFrame(nsIFrame*) layout/generic/nsTextFrameThebes.cpp:1588
    #3 0x7fae4a6878e3 in BuildTextRuns(gfxContext*, nsTextFrame*, nsIFrame*, nsLineList_iterator const*, nsTextFrame::TextRunType) layout/generic/nsTextFrameThebes.cpp:1315
    #4 0x7fae4a6832f6 in nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, gfxContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) layout/generic/nsTextFrameThebes.cpp:2465
    #5 0x7fae4a7007d7 in nsTextFrame::ReflowText(nsLineLayout&, int, nsRenderingContext*, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsTextFrameThebes.cpp:7664
    #6 0x7fae4a5283bd in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:837
    #7 0x7fae4a1c6659 in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:3700
    #8 0x7fae4a1c130e in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3497
    #9 0x7fae4a1b5222 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3351
    #10 0x7fae4a1a5b32 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:2492
    #11 0x7fae4a18f78d in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2011
    #12 0x7fae4a182edc in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1010
    #13 0x7fae4a205d78 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) layout/generic/nsBlockReflowContext.cpp:266
    #14 0x7fae4a1aff52 in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3078
    #15 0x7fae4a1a5877 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:2489
    #16 0x7fae4a18f78d in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2011
    #17 0x7fae4a182edc in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1010
    #18 0x7fae4a2701b9 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #19 0x7fae4a241bd7 in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsCanvasFrame.cpp:487
    #20 0x7fae4a2701b9 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #21 0x7fae4a3d0d91 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:445
    #22 0x7fae4a3d4f10 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) layout/generic/nsGfxScrollFrame.cpp:545
    #23 0x7fae4a3d93d5 in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsGfxScrollFrame.cpp:786
    #24 0x7fae4a2701b9 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:970
    #25 0x7fae4a7be24e in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsViewportFrame.cpp:225
    #26 0x7fae49ef4f12 in PresShell::DoReflow(nsIFrame*, bool) layout/base/nsPresShell.cpp:7845
    #27 0x7fae49f223c1 in PresShell::ProcessReflowCommands(bool) layout/base/nsPresShell.cpp:7986
    #28 0x7fae49f20b7d in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) layout/base/nsPresShell.cpp:3897
    #29 0x7fae49f1f1d7 in PresShell::FlushPendingNotifications(mozFlushType) layout/base/nsPresShell.cpp:3743
    #30 0x7fae49d116cf in nsDocumentViewer::LoadComplete(tag_nsresult) layout/base/nsDocumentViewer.cpp:985
    #31 0x7fae51c12584 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, tag_nsresult) docshell/base/nsDocShell.cpp:6684
    #32 0x7fae51c098f0 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) docshell/base/nsDocShell.cpp:6481
    #33 0x7fae51c0a819 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) docshell/base/nsDocShell.cpp:6488
    #34 0x7fae51cdf367 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, tag_nsresult) uriloader/base/nsDocLoader.cpp:1323
    #35 0x7fae51cdca8c in nsDocLoader::doStopDocumentLoad(nsIRequest*, tag_nsresult) uriloader/base/nsDocLoader.cpp:865
    #36 0x7fae51cd5c14 in nsDocLoader::DocLoaderIsEmpty(bool) uriloader/base/nsDocLoader.cpp:755
    #37 0x7fae51cd9ecb in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) uriloader/base/nsDocLoader.cpp:639
    #38 0x7fae51cdb58a in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) uriloader/base/nsDocLoader.cpp:643
    #39 0x7fae47d79e10 in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, tag_nsresult) netwerk/base/src/nsLoadGroup.cpp:684
    #40 0x7fae4bb2d445 in nsDocument::DoUnblockOnload() content/base/src/nsDocument.cpp:7966
    #41 0x7fae4bb2ce9b in nsDocument::UnblockOnload(bool) content/base/src/nsDocument.cpp:7894
    #42 0x7fae4bacee03 in nsDocument::DispatchContentLoadedEvents() content/base/src/nsDocument.cpp:4676
    #43 0x7fae4bbee32a in nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() ../../../dist/include/nsThreadUtils.h:350
    #44 0x7fae581a88a7 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:626
    #45 0x7fae57e12c02 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
    #46 0x7fae53eabb49 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
    #47 0x7fae5852b9cb in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:219
    #48 0x7fae5852b81e in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:212
    #49 0x7fae5852b709 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:186
    #50 0x7fae53698c3e in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
    #51 0x7fae5221e920 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:269
    #52 0x7fae471007ab in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3856
    #53 0x7fae4710555d in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3924
    #54 0x7fae47108195 in XRE_main toolkit/xre/nsAppRunner.cpp:4126
    #55 0x42a0d7 in do_main(int, char**, nsIFile*) browser/app/nsBrowserApp.cpp:272
    #56 0x4270e1 in main browser/app/nsBrowserApp.cpp:632
    #57 0x7fae6ade576c in
    #58 0x426854 in
0x61902a674520 is located 0 bytes to the right of 32-byte region [0x61902a674500,0x61902a674520)
allocated by thread T0 here:
    #0 0x41a6d2 in malloc
    #1 0x7fae68d41825 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54
    #2 0x7fae5826ca5e in NS_Alloc xpcom/base/nsMemoryImpl.cpp:198
    #3 0x7fae57dfa810 in nsMemory::Clone(void const*, unsigned long) objdir-ff-asan-sym/xpcom/build/nsMemory.cpp:29
    #4 0x7fae4c016444 in nsTextFragment::SetTo(unsigned short const*, int, bool) content/base/src/nsTextFragment.cpp:246
    #5 0x7fae4bd858c8 in nsGenericDOMDataNode::SetTextInternal(unsigned int, unsigned int, unsigned short const*, unsigned int, bool, CharacterDataChangeInfo::Details*) content/base/src/nsGenericDOMDataNode.cpp:329
    #6 0x7fae4bd8d25f in nsGenericDOMDataNode::SplitData(unsigned int, nsIContent**, bool) content/base/src/nsGenericDOMDataNode.cpp:723
    #7 0x7fae4b6dafd7 in mozilla::dom::Text::SplitText(unsigned int, mozilla::ErrorResult&) content/base/src/Text.cpp:16
    #8 0x7fae5706f6e8 in mozilla::dom::TextBinding::splitText(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Text*, JSJitMethodCallArgs const&) objdir-ff-asan-sym/dom/bindings/TextBinding.cpp:34
    #9 0x7fae5706ed2b in mozilla::dom::TextBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/TextBinding.cpp:105
    #10 0x7fae5f99a51c in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:321
    #11 0x7fae5f99a51c in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:479
    #12 0x7fae5f9796cf in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2294
    #13 0x7fae5f92801e in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:436
    #14 0x7fae5f99aba4 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:498
    #15 0x7fae5f99ea9f in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/vm/Interpreter.cpp:529
    #16 0x7fae5ffbd5c8 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5767
    #17 0x7fae5176d7b5 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1436
    #18 0x7fae51739e0b in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:589
    #19 0x7fae582d7934 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
    #20 0x7fae582d49ea in SharedStub
    #21 0x7fae4c9c051c in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:941
    #22 0x7fae4c9c2c4f in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:1012
    #23 0x7fae4c9a7a5b in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.h:328
    #24 0x7fae4c9976c7 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:221
    #25 0x7fae4c995176 in nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:342
    #26 0x7fae4c99cce7 in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) content/events/src/nsEventDispatcher.cpp:644
    #27 0x7fae4c99f38d in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) content/events/src/nsEventDispatcher.cpp:704
    #28 0x7fae4bdd9834 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) content/base/src/nsINode.cpp:1148
    #29 0x7fae4b86b1eb in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3300
    #30 0x7fae4b86a554 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3270
Shadow bytes around the buggy address:
  0x0c32854c6850: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c6860: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c6870: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c6880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c6890: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c32854c68a0: 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c68b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c68c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c68d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c68e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32854c68f0: 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
==1606==ABORTING

Comment 1

[Jonathan Kew [:jfkthame]]

FWIW, when I load this testcase in a debug build on OS X, it fires an assertion here:

###!!! 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

I don't know whether this is connected with the ASAN error above, though.

Comment 2

[Jesse Ruderman]

I suspect bug 653881.

Why does <xbl:children> do anything special in content?  I thought we banished XBL.

Comment 3

[Jesse Ruderman]

Bug 890775 has a simpler testcase for just the assertion.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Jesse Ruderman from comment #2)
> I suspect bug 653881.

Right on.

> Why does <xbl:children> do anything special in content?  I thought we
> banished XBL.

We did. The patch that landed half-unbanished it. The patch in bug 890775 fixes this testcase by making xbl:children not in a binding act like a regular element again.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

This should be fixed now.

Comment 6

[Daniel Veditz [:dveditz]]

mrbkap: care to take a guess at the severity rating for this one? The crash here looks like it's reading bogus text; how badly are we confused at this point? I realize it's somewhat moot because we've fixed it, but

Comment 7

[Daniel Veditz [:dveditz]]

... but we need to evaluate this for a Bug Bounty.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

It's very hard for me to make a judgement here. I don't know the code nearly well enough, however, given how similar this testcase is to the testcase in bug 890769 it's probably sg:crit. Jonathan, does that sound reasonable to you?

Comment 9

[Jonathan Kew [:jfkthame]]

The testcase here does not look critical in itself, given that it's only -reading- text; it's checking for the presence of specific characters in the text represented by the textrun object. Overrunning the textrun and reading junk values might cause us to make incorrect line-breaking or other layout decisions, but that's pretty benign.

However, the underlying issue is that we have a mismatch between the length of the text in the content, and the length of the textrun that we think corresponds to it. If we were to -write- rather than just -read- past the end of the textrun, we'd potentialy be corrupting something else on the heap.

And that's what bug 890769 indicates: there, we have the same fundamental issue of the textrun length not matching the content text, but then we go into line-breaking, which will update the line-break flags in the textrun. So in that bug, if ASAN hadn't killed us immediately on the out-of-bounds read, we would have proceeded to do a write to the same out-of-bounds location. And probably to some following ones, depending on the lengths involved.

I'd generally assume such an out-of-bounds write must be sg:crit. In the textrun line-breaking case, it is perhaps mitigated somewhat by the fact that the line-breaking process only modifies two -bits- within each 32-bit CompressedGlyph record that it touches; it does not have the opportunity to overwrite entire bytes or words arbitrarily. ISTM that might severely limit what an attacker could achieve through this vector - but then, I'm pretty ignorant of how things like this are exploited, and I'm sure the bad guys often manage to achieve things I would never have dreamed of. So I guess bug 890769 should probably be considered sg:crit.

As for this one: I don't think it looks critical in itself, but I think it's really a dupe of bug 890769. The underlying failure exposed by both testcases is the same: the textrun length mismatch, triggered by involving <xbl:children> in mutating the document, or something like that.

Comment 11

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

Confirmed original assertion FF25, 2013-07-11.
Verified fixed FF25, 2013-10-01.
Same as related bugs 890769 and 890775.