Closed Bug 1288946 (CVE-2016-5271) Opened 8 years ago Closed 8 years ago

Out-of-bounds read in PropertyProvider::GetSpacingInternal

Categories

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

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla51
Tracking Status
firefox48 --- wontfix
firefox49 --- verified
firefox-esr45 --- wontfix
thunderbird_esr38 --- wontfix
thunderbird_esr45 --- wontfix
firefox50 --- verified
firefox51 --- verified

People

(Reporter: inferno, Assigned: MatsPalmgren_bugz)

Details

(5 keywords, Whiteboard: [adv-main49+])

Attachments

(5 files)

Attached file Testcase
==5132==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d0001bcf1f at pc 0x7fc40183ee4f bp 0x7ffd840697d0 sp 0x7ffd840697c8 READ of size 1 at 0x60d0001bcf1f thread T0 (Web Content) SCARINESS: 12 (1-byte-read-heap-buffer-overflow) #0 0x7fc40183ee4e in CharAt dom/base/nsTextFragment.h:204:68 #1 0x7fc40183ee4e in IsCSSWordSpacingSpace layout/generic/nsTextFrame.cpp:642 #2 0x7fc40183ee4e in PropertyProvider::GetSpacingInternal(gfxTextRun::Range, gfxFont::Spacing*, bool) layout/generic/nsTextFrame.cpp:3214 #3 0x7fc3fc9bacab in GetAdjustedSpacing gfx/thebes/gfxTextRun.cpp:343:16 #4 0x7fc3fc9bacab in gfxTextRun::BreakAndMeasureText(unsigned int, unsigned int, bool, double, gfxTextRun::PropertyProvider*, gfxTextRun::SuppressBreak, double*, gfxFont::RunMetrics*, gfxFont::BoundingBoxType, mozilla::gfx::DrawTarget*, bool*, unsigned int*, bool, gfxBreakPriority*) gfx/thebes/gfxTextRun.cpp:857 #5 0x7fc40187ac0a in nsTextFrame::ReflowText(nsLineLayout&, int, mozilla::gfx::DrawTarget*, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsTextFrame.cpp:8969:15 #6 0x7fc40157d573 in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:946:7 #7 0x7fc4017b221e in nsInlineFrame::ReflowInlineFrame(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsIFrame*, unsigned int&) layout/generic/nsInlineFrame.cpp:805:15 #8 0x7fc4017b1088 in nsInlineFrame::ReflowFrames(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsInlineFrame.cpp:689:7 #9 0x7fc4017afe96 in nsInlineFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsInlineFrame.cpp:466:3 #10 0x7fc40157d4dc in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:943:13 #11 0x7fc4015f2a0b in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:4088:15 #12 0x7fc4015f12ef in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3890:5 #13 0x7fc4015e7cbf in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3756:9 #14 0x7fc4015d8433 in ReflowLine layout/generic/nsBlockFrame.cpp:2754:5 #15 0x7fc4015d8433 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2290 #16 0x7fc4015d132c in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1171:3 #17 0x7fc40162f3dc in ReflowChild layout/generic/nsContainerFrame.cpp:1022:14 #18 0x7fc40162f3dc in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsCanvasFrame.cpp:644 #19 0x7fc4016d6c16 in ReflowChild layout/generic/nsContainerFrame.cpp:1022:14 #20 0x7fc4016d6c16 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:540 #21 0x7fc4016d807d in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) layout/generic/nsGfxScrollFrame.cpp:652:3 #22 0x7fc4016da4a1 in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsGfxScrollFrame.cpp:886:3 #23 0x7fc40164724f in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:1065:14 #24 0x7fc401895f47 in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsViewportFrame.cpp:314:7 #25 0x7fc401502057 in PresShell::DoReflow(nsIFrame*, bool) layout/base/nsPresShell.cpp:9597:11 #26 0x7fc401515d81 in PresShell::ProcessReflowCommands(bool) layout/base/nsPresShell.cpp:9770:24 #27 0x7fc401515011 in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) layout/base/nsPresShell.cpp:4142:11 #28 0x7fc401708450 in mozilla::ScrollFrameHelper::AsyncScrollPortEvent::Run() layout/generic/nsGfxScrollFrame.cpp:4632:7 #29 0x7fc4014eec2f in nsRootPresContext::FlushWillPaintObservers() layout/base/nsPresContext.cpp:3098:19 #30 0x7fc401570ef4 in nsRootPresContext::RunWillPaintObservers::Run() layout/base/nsPresContext.h:1543:23 #31 0x7fc3fa142b62 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1068:14 #32 0x7fc3fa1c2878 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:290:10 #33 0x7fc3faf28ec1 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:100:21 #34 0x7fc3fae9b6b0 in RunInternal ipc/chromium/src/base/message_loop.cc:232:10 #35 0x7fc3fae9b6b0 in RunHandler ipc/chromium/src/base/message_loop.cc:225 #36 0x7fc3fae9b6b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:205 #37 0x7fc400b66e2f in nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:156:27 #38 0x7fc402cb1237 in XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:829:22 #39 0x7fc3fae9b6b0 in RunInternal ipc/chromium/src/base/message_loop.cc:232:10 #40 0x7fc3fae9b6b0 in RunHandler ipc/chromium/src/base/message_loop.cc:225 #41 0x7fc3fae9b6b0 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:205 #42 0x7fc402cb08f8 in XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:659:34 #43 0x510ad9 in content_process_main ipc/contentproc/plugin-container.cpp:224:19 #44 0x510ad9 in main browser/app/nsBrowserApp.cpp:347 #45 0x7fc413dd2f44 in __libc_start_main /build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287 0x60d0001bcf1f is located 0 bytes to the right of 79-byte region [0x60d0001bced0,0x60d0001bcf1f) allocated by thread T0 (Web Content) here: #0 0x4d4ed8 in malloc _asan_rtl_ #1 0x7fc3fd18f349 in nsTextFragment::SetTo(char16_t const*, int, bool) dom/base/nsTextFragment.cpp:281:37 #2 0x7fc3fd083a2b in nsGenericDOMDataNode::SetTextInternal(unsigned int, unsigned int, char16_t const*, unsigned int, bool, CharacterDataChangeInfo::Details*) dom/base/nsGenericDOMDataNode.cpp:333:21 #3 0x7fc3fd0890ae in nsGenericDOMDataNode::SetText(char16_t const*, unsigned int, bool) dom/base/nsGenericDOMDataNode.cpp:994:10 #4 0x7fc3fc277f54 in nsHtml5TreeOperation::AppendText(char16_t const*, unsigned int, nsIContent*, nsHtml5DocumentBuilder*) parser/html/nsHtml5TreeOperation.cpp:164:14 #5 0x7fc3fc266f43 in nsHtml5TreeBuilder::appendCharacters(void*, char16_t*, int, int) parser/html/nsHtml5TreeBuilderCppSupplement.h:561:19 #6 0x7fc3fc20dad1 in nsHtml5TreeBuilder::flushCharacters() parser/html/nsHtml5TreeBuilder.cpp:4263:5 #7 0x7fc3fc2564f8 in nsHtml5TreeBuilder::eof() parser/html/nsHtml5TreeBuilder.cpp:478:3 #8 0x7fc3fc20990b in nsHtml5Tokenizer::eof() parser/html/nsHtml5Tokenizer.cpp:3849:17 #9 0x7fc3fc212ae9 in nsHtml5StringParser::Tokenize(nsAString_internal const&, nsIDocument*, bool) parser/html/nsHtml5StringParser.cpp:124:17 #10 0x7fc3fcbe8bc6 in nsContentUtils::ParseFragmentHTML(nsAString_internal const&, nsIContent*, nsIAtom*, int, bool, bool) dom/base/nsContentUtils.cpp:4518:26 #11 0x7fc3fcdcd668 in mozilla::dom::Element::SetOuterHTML(nsAString_internal const&, mozilla::ErrorResult&) dom/base/Element.cpp:3508:5 #12 0x7fc3fe9b3997 in mozilla::dom::ElementBinding::set_outerHTML(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitSetterCallArgs) objdir-ff-asan/dom/bindings/ElementBinding.cpp:3067:9 #13 0x7fc3fedccf89 in mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp:2752:8 #14 0x7fc404e87aa3 in CallJSNative js/src/jscntxtinlines.h:232:15 #15 0x7fc404e87aa3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:453 #16 0x7fc404e89a79 in InternalCall js/src/vm/Interpreter.cpp:498:12 #17 0x7fc404e89a79 in Call js/src/vm/Interpreter.cpp:517 #18 0x7fc404e89a79 in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) js/src/vm/Interpreter.cpp:644 #19 0x7fc404eeeb0e in SetExistingProperty js/src/vm/NativeObject.cpp:2364:10 #20 0x7fc404eeeb0e in js::NativeSetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::QualifiedBool, JS::ObjectOpResult&) js/src/vm/NativeObject.cpp:2399 #21 0x7fc404e6a96e in SetProperty js/src/vm/NativeObject.h:1495:12 #22 0x7fc404e6a96e in SetPropertyOperation js/src/vm/Interpreter.cpp:256 #23 0x7fc404e6a96e in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2666 #24 0x7fc404e586f8 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:399:12 #25 0x7fc404e8a2f3 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:679:15 #26 0x7fc404e8aa00 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:711:12 #27 0x7fc4049ad5ab in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::StaticScope*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4405:19 #28 0x7fc4049ae0e6 in Evaluate js/src/jsapi.cpp:4432:12 #29 0x7fc4049ae0e6 in JS::Evaluate(JSContext*, JS::AutoVectorRooter<JSObject*>&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4493 #30 0x7fc3fd0d72f0 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) dom/base/nsJSUtils.cpp:206:12 #31 0x7fc3fd0d80a7 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) dom/base/nsJSUtils.cpp:266:10 #32 0x7fc3fd176995 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*) dom/base/nsScriptLoader.cpp:2010:12 #33 0x7fc3fd17334b in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*) dom/base/nsScriptLoader.cpp:1808:10 #34 0x7fc3fd15bf2b in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) dom/base/nsScriptLoader.cpp:1546:10 #35 0x7fc3fd158293 in nsScriptElement::MaybeProcessScript() dom/base/nsScriptElement.cpp:141:18 #36 0x7fc3fc272285 in AttemptToExecute dom/base/nsIScriptElement.h:221:18 #37 0x7fc3fc272285 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) parser/html/nsHtml5TreeOpExecutor.cpp:664 SUMMARY: AddressSanitizer: heap-buffer-overflow (/mnt/scratch0/clusterfuzz/slave-bot/builds/linux_asan_firefox/custom/firefox/libxul.so+0x95fde4e) Shadow bytes around the buggy address: 0x0c1a8002f990: 00 00 00 00 02 fa fa fa fa fa fa fa fa fa fa fa 0x0c1a8002f9a0: fa fa fa fa fa fa 00 00 00 00 00 00 00 00 00 fa 0x0c1a8002f9b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a8002f9c0: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa 0x0c1a8002f9d0: fa fa fa fa fa fa fa fa fa fa 00 00 00 00 00 00 =>0x0c1a8002f9e0: 00 00 00[07]fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a8002f9f0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fa fa 0x0c1a8002fa00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 00 0x0c1a8002fa10: 00 00 00 00 00 00 02 fa fa fa fa fa fa fa fa fa 0x0c1a8002fa20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x0c1a8002fa30: 00 02 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 right 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 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==5132==ABORTING
jfkthame: what is the code doing at this point? will this data just be read in and result in a bogus text measurement (bad layout, no big deal), or do we have corrupted memory that could lead to worse things?
Group: core-security → layout-core-security
Component: Layout → Layout: Text
Flags: needinfo?(jfkthame)
Well... the specific out-of-bounds read here is unlikely to be a serious problem in itself, but I'd be concerned that it might indicate a deeper problem -- e.g. something like using a textrun that has not been updated to reflect changes in the underlying DOM text -- which might have the potential to lead to worse problems further on. I tried loading the testcase here in a debug build, and it hits an assertion in nsCSSFrameConstructor: Assertion failure: aInsertion->mParentFrame == fakeInsertion.mParentFrame, at /Users/jkew/mozdev/mozilla-central/layout/base/nsCSSFrameConstructor.cpp:6765 #01: nsCSSFrameConstructor::GetInsertionPrevSibling(nsCSSFrameConstructor::InsertionPoint*, nsIContent*, bool*, bool*, nsIContent*, nsIContent*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4ec32e4] #02: nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4ec5086] #03: nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent*, nsILayoutHistoryState*, bool)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4ec3e08] #04: PresShell::ContentInserted(nsIDocument*, nsIContent*, nsIContent*, int)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4f90dd9] #05: nsNodeUtils::ContentInserted(nsINode*, nsIContent*, int)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x258fcaf] #06: nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x254cf0f] #07: mozilla::dom::FragmentOrElement::InsertChildAt(nsIContent*, unsigned int, bool)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x238eb38] #08: nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x254fe1e] #09: nsINode::ReplaceChild(nsINode&, nsINode&, mozilla::ErrorResult&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x23720b4] #10: mozilla::dom::Element::SetOuterHTML(nsAString_internal const&, mozilla::ErrorResult&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2371dda] #11: mozilla::dom::ElementBinding::set_outerHTML(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitSetterCallArgs)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3446f3a] #12: mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x366d282] #13: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79c7a6d] #14: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79c77de] #15: InternalCall(JSContext*, js::AnyInvokeArgs const&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79c83b7] #16: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x799e856] #17: js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79c8f19] #18: SetExistingProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<js::Shape*>, JS::ObjectOpResult&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a2c3a8] #19: js::NativeSetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::QualifiedBool, JS::ObjectOpResult&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a2ba2e] #20: js::SetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79ecb6f] #21: SetPropertyOperation(JSContext*, JSOp, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::Handle<JS::Value>)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79ec9ca] #22: Interpret(JSContext*, js::RunState&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79bb05a] #23: js::RunScript(JSContext*, js::RunState&)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79b2252] #24: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79c928b] #25: js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79c98c3] #26: Evaluate(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::StaticScope*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x76d2f40] #27: Evaluate(JSContext*, JS::AutoVectorRooter<JSObject*>&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x76d329f] #28: JS::Evaluate(JSContext*, JS::AutoVectorRooter<JSObject*>&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x76d318d] #29: nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2564b93] #30: nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2565102] #31: nsScriptLoader::EvaluateScript(nsScriptLoadRequest*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x25c8de9] #32: nsScriptLoader::ProcessRequest(nsScriptLoadRequest*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x25c73ce] #33: nsScriptLoader::ProcessScriptElement(nsIScriptElement*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x25bcc96] #34: nsScriptElement::MaybeProcessScript()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x25bb446] #35: nsIScriptElement::AttemptToExecute()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1b1c60e] #36: nsHtml5TreeOpExecutor::RunScript(nsIContent*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1b1bd7b] #37: nsHtml5TreeOpExecutor::RunFlushLoop()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1b1b4aa] #38: nsHtml5ExecutorFlusher::Run()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1b1df41] #39: nsThread::ProcessNextEvent(bool, bool*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1b4bb9] #40: NS_ProcessPendingEvents(nsIThread*, unsigned int)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x23aa5c] #41: nsBaseAppShell::NativeEventCallback()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x49e31ee] #42: nsAppShell::ProcessGeckoEvents(void*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4a7aa12] #43: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x7f5b1] #44: __CFRunLoopDoSources0[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x70c62] #45: __CFRunLoopRun[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x703ef] #46: CFRunLoopRunSpecific[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x6fe75] #47: RunCurrentEventLoopInMode[/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x2ea0d] #48: ReceiveNextEventCommon[/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x2e7b7] #49: _BlockUntilNextEventMatchingListInModeWithFilter[/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x2e5bc] #50: _DPSNextEvent[/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x2424e] #51: -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:][/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x2389b] #52: -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:][/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4a79564] #53: -[NSApplication run][/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x1799c] #54: nsAppShell::Run()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4a7b3ac] #55: nsAppStartup::Run()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5dfb83b] #56: XREMain::XRE_mainRun()[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5efdd11] #57: XREMain::XRE_main(int, char**, nsXREAppData const*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5efeb31] #58: XRE_main[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5eff1f2] #59: do_main(int, char**, char**, nsIFile*)[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/firefox +0x1e5a] #60: main[/Users/jkew/mozdev/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/firefox +0x1322] That seems worrying; are we getting confused somehow during frame construction? cc'ing a few people who might be more familiar with nsCSSFrameConstructor...
Flags: needinfo?(jfkthame)
I started looking at this, but I'm not familiar enough with frame construction code to have worked out the problem yet. I get the same assertion that Jonathan got, and that at least seems to be related to display:contents. In attachment 8779665 [details], this is the content tree I have just when we hit the assertion (i.e. in the middle of the insertBefore() call) when I run the test with |./firefox ... -chrome test.html|: html@0x7f2eb3562240 state=[40000020000] flags=[00100000] primaryframe=0x7f2eac87d528 refcount=13< head@0x7f2eb35fd500 state=[40000020000] flags=[00100000] primaryframe=(nil) refcount=3<> body@0x7f2eb37f5ba0 state=[40000020000] flags=[00100000] primaryframe=0x7f2eac87dc00 refcount=4< strong@0x7f2eb47a5f20 state=[40000020000] flags=[00100000] primaryframe=0x7f2eac87f2a8 refcount=6< Text@0x7f2eb47a5fb0 flags=[08000000] primaryframe=0x7f2eac87f418 refcount=3<a b> span@0x7f2eb47a6040 id="span" style="display: contents;" state=[40000020000] flags=[00100000] primaryframe=(nil) refcount=7< small@0x7f2eb47a61f0 state=[40000020000] flags=[00100000] primaryframe=(nil) refcount=2<> script@0x7f2eb35168c0 state=[40000020000] flags=[00100000] primaryframe=(nil) refcount=15< Text@0x7f2eb47a60d0 flags=[00000000] primaryframe=(nil) refcount=1<...> > > > > > And this is the frame tree: Viewport(-1)@7f2eb35a6968 [view=7f2eb3539800] {0,0,60,60} [state=000b063000003220] [sc=7f2eb35a65a0:-moz-viewport^0]< HTMLScroll(html)(-1)@7f2eb35a74e8 {0,0,60,60} [state=000b020000001010] [content=7f2eb3562240] [sc=7f2eb35a6140:-moz-viewport-scroll]< ScrollbarFrame(scrollbar)(-1)@7f2eb35a7d20 next=7f2eac87b7f0 {0,60,60,0} [state=000b100080c80000] [content=7f2eb47a5230] [sc=7f2eb35a6d38]< SliderFrame(slider)(-1)@7f2eac87aab0 {120,0,0,240} vis-overflow=0,0,1800,240 scr-overflow=0,0,1800,240 [state=000b160080c00000] [content=7f2eb47a5590] [sc=7f2eac87a710]< ButtonBoxFrame(thumb)(0)@7f2eac87b238 {0,120,1800,0} [state=2009160080400000] [content=7f2eb47a5620] [sc=7f2eac87ae18]<> > > ScrollbarFrame(scrollbar)(-1)@7f2eac87b7f0 next=7f2eac87cdf8 {60,0,0,60} [state=000b100080880000] [content=7f2eb47a52c0] [sc=7f2eac87b508]< SliderFrame(slider)(-1)@7f2eac87c308 {0,120,240,0} vis-overflow=0,0,240,1800 scr-overflow=0,0,240,1800 [state=000b160080800000] [content=7f2eb47a5aa0] [sc=7f2eac87c020]< ButtonBoxFrame(thumb)(0)@7f2eac87c778 {120,0,0,1800} [state=2009160080000000] [content=7f2eb47a5b30] [sc=7f2eac87c490]<> > > Box(scrollcorner)(-1)@7f2eac87cdf8 next=7f2eb35a7170 {60,60,0,0} [state=0009100080c00200] [content=7f2eb47a5350] [sc=7f2eac87c9d8]<> Canvas(html)(-1)@7f2eb35a7170 {0,0,60,60} vis-overflow=0,0,1140,3240 scr-overflow=0,0,1140,3240 [state=000b002000001200] [content=7f2eb3562240] [sc=7f2eac87d018:-moz-scrolled-canvas]< Block(html)(-1)@7f2eac87d528 {0,0,60,3240} vis-overflow=0,0,1440,3240 scr-overflow=0,0,1140,3240 [state=200b100000d01200] [content=7f2eb3562240] [sc=7f2eac87d468^0]< line 7f2eac87dc98: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x48] bm=480 {480,480,0,2280} vis-overflow=480,480,960,2280 scr-overflow=480,480,660,2280 < Block(body)(1)@7f2eac87dc00 {480,480,0,2280} vis-overflow=0,0,960,2280 scr-overflow=0,0,660,2280 [state=200b120000101200] [content=7f2eb37f5ba0] [sc=7f2eac87d9e8]< line 7f2eac87f320: count=1 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {0,0,600,1140} vis-overflow=0,0,960,1140 scr-overflow=0,0,600,1140 < Inline(strong)(0)@7f2eac87f2a8 next=7f2eac87f7d8 next-in-flow=7f2eac87f7d8 {0,0,600,1140} vis-overflow=0,0,960,1140 [state=0002100000000200] [content=7f2eb47a5f20] [sc=7f2eac87db50]< Text(0)"a "@7f2eac87f418 next-in-flow=7f2eac8807c0 {0,0,600,1140} vis-overflow=0,0,960,1140 [state=00010000b1600000] [content=7f2eb47a5fb0] [sc=7f2eac87f370:-moz-text] [run=7f2eb47ace80][0,2,F] > > line 7f2eac880848: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x100] {0,1140,660,1140} < Inline(strong)(0)@7f2eac87f7d8 prev-in-flow=7f2eac87f2a8 {0,1140,660,1140} [state=0002100000001204] [content=7f2eb47a5f20] [sc=7f2eac87db50]< Text(0)"b"@7f2eac8807c0 prev-in-flow=7f2eac87f418 {0,0,660,1140} [state=0001000090600004] [content=7f2eb47a5fb0] [sc=7f2eac87f370:-moz-text] [run=7f2eb47ace80][2,1,T] > > > > > > > > So the <span> is display:contents, and doesn't have a frame. We're inserting the <small> as a child of the <span> (before the <strong>'s existing child, the <script>). We've called nsCSSFrameConstructor::GetInsertionPrevSibling to look for the frame that we'll eventually pass to InsertFrames (so that our <small>'s frame gets put in the right place in the frame tree). We first try looking for a preceding sibling element's frame, but we find that we don't have a preceding sibling element. http://searchfox.org/mozilla-central/rev/d83f1528dbcb1e837d99cf5b6c36a90b03bf0e77/layout/base/nsCSSFrameConstructor.cpp#6730-6731 Then we try looking for a following sibling element's frame, but the <script> doesn't have a frame, and then we reach the end of the child list. http://searchfox.org/mozilla-central/rev/d83f1528dbcb1e837d99cf5b6c36a90b03bf0e77/layout/base/nsCSSFrameConstructor.cpp#6744-6745 Without display:contents being involved, we would then decide to append our new element's frame to the parent. But since display:contents is involved, we instead have to recursively call GetInsertionPrevSibling pretending that we're inserting the <span> into its parent, the <strong>. http://searchfox.org/mozilla-central/rev/d83f1528dbcb1e837d99cf5b6c36a90b03bf0e77/layout/base/nsCSSFrameConstructor.cpp#6752-6760 When the assertion fails: MOZ_ASSERT(aInsertion->mParentFrame == fakeInsertion.mParentFrame); we have aInsertion->mParentFrame being the <strong>'s frame 0x7f2eac87f2a8, and fakeInsertion.mParentFrame being the <strong>'s frame's continuation 0x7f2eac87f7d8. I don't know whether that is something that makes sense or not, or what failing this assertion entails and whether it relates to the ASAN assertion. I might need Mats' help when he's back from PTO.
FWIW if I comment out the assertion, my reduced test triggers a couple more assertions, but not the ASAN assertion: [4316] ###!!! ASSERTION: Unexpected frame types: '(frame1->GetType() == nsGkAtoms::tableFrame && frame2->GetType() == nsGkAtoms::tableWrapperFrame) || (frame1->GetType() == nsGkAtoms::tableWrapperFrame && frame2->GetType() == nsGkAtoms::tableFrame) || frame1->GetType() == nsGkAtoms::fieldSetFrame || (frame1->GetParent() && frame1->GetParent()->GetType() == nsGkAtoms::fieldSetFrame)', file /z/moz/central/layout/base/nsCSSFrameConstructor.cpp, line 8009 [4316] ###!!! ASSERTION: aParentFrame has later continuations with kids?: 'nextSibling || !aParentFrame->GetNextContinuation() || !aParentFrame->GetNextContinuation()->PrincipalChildList().FirstChild() || aIsRecursiveCall', file /z/moz/central/layout/base/nsCSSFrameConstructor.cpp, line 6390 [4316] ###!!! ASSERTION: aPrevFrame must be the last continuation in its chain!: '!aPrevFrame || (!aPrevFrame->GetNextContinuation() || (((aPrevFrame->GetNextContinuation()->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)) && !(aPrevFrame->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)))', file /z/moz/central/layout/base/nsFrameManager.cpp, line 471
Mats, can you take a look at this when you return?
Flags: needinfo?(mats)
Sure, I'm looking at this now...
We first do an out-of-bounds read in IsCSSWordSpacingSpace, and then later in IsTrimmableSpace. If I add an early return for out-of-bounds indices in those then there are no further complaints from ASAN. So after investigating this I'm leaning towards sec-low. (FTR, I tried both returning false/true.) I tried some variations of the testcase as well but couldn't trigger any new issues. There's a lot fragile textframe/textrun code involved though so I'd recommend uplifting the fix to branches anyway, as a precaution.
Flags: needinfo?(mats)
Keywords: sec-low
Attached patch fixSplinter Review
The nested GetInsertionPrevSibling call finds a suitable sibling in a continuation of the mParentFrame we give it. We should use that frame for the child frame insertion. We're currently using the first- in-flow no matter what and this results in the frame for the inserted content being inserted in between continuations for the sibling. This apparently makes the TextFrame code confused about indices into the TextRun. In the original testcase we insert a textnode (T), and the three text frames end up sharing the same text run. Like so: frame1(sibling) -> frame(T) -> frame2(sibling) |-------- next-in-flow --------^
Assignee: nobody → mats
Attachment #8781658 - Flags: review?(cam)
Attached patch crashtest 1Splinter Review
This is the original testcase, it asserts in DEBUG builds and crashes an ASAN build.
Attached patch crashtest 2Splinter Review
Two tests based on heycam's testcase. I added <body style="width:0"> to trigger continuations for the text. Also added a textnode child of the inserted element (<small> in the -2a test and <span> in -2b). The latter shares the textrun - which triggers additional assertions: ASSERTION: Invalid offset: 'uint32_t(aOffset) <= mSkipChars->mCharCount', file gfx/thebes/gfxSkipChars.cpp, line 23 ASSERTION: Text run does not map enough text for our reflow: 'gfxSkipCharsIterator(iter).ConvertOriginalToSkipped(offset + length) <= mTextRun->GetLength()', file layout/generic/nsTextFrame.cpp, line 9036 (neither of these tests crash in an ASAN build though)
FWIW, the bug only affects pages that use display:contents.
display:contents was enabled by default in v37 (bug 1105369), so all branches are affected.
Severity: normal → critical
Flags: in-testsuite?
OS: Unspecified → All
Hardware: Unspecified → All
Attachment #8781658 - Flags: review?(cam) → review+
Comment on attachment 8781661 [details] [diff] [review] crashtest 2 Review of attachment 8781661 [details] [diff] [review]: ----------------------------------------------------------------- r=me but you can remove the |document.implementation;| line -- that was just there for me to have a handy function to add a breakpoint on.
Attachment #8781661 - Flags: review+
I guess we want to hold off checking those tests in, though.
Yes, I'm holding the tests until the bug is public.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment on attachment 8781658 [details] [diff] [review] fix Approval Request Comment [Feature/regressing bug #]: bug 907396 [User impact if declined]: crash [Describe test coverage new/current, TreeHerder]: will add crashtests when the bug is public [Risks and why]: low - only affects content styled with display:contents [String/UUID change made/needed]: none The particular crash for these testcases seems unlikely to be exploitable but I'm slightly worried that other variations might lead to more serious issues so I think taking this low-risk fix on branches is warranted.
Attachment #8781658 - Flags: approval-mozilla-esr45?
Attachment #8781658 - Flags: approval-mozilla-beta?
Attachment #8781658 - Flags: approval-mozilla-aurora?
Comment on attachment 8781658 [details] [diff] [review] fix Given this may cover a wider range of problems, let's uplift to aurora and beta 6.
Attachment #8781658 - Flags: approval-mozilla-beta?
Attachment #8781658 - Flags: approval-mozilla-beta+
Attachment #8781658 - Flags: approval-mozilla-aurora?
Attachment #8781658 - Flags: approval-mozilla-aurora+
Comment on attachment 8781658 [details] [diff] [review] fix Doesn't match the esr criteria. If you think it is more than sec-low, please change the sec evaluation.
Attachment #8781658 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45-
Group: layout-core-security → core-security-release
Whiteboard: [adv-main49+]
Alias: CVE-2016-5271
Reproduced the crash on 50.0a1 (2016-07-24) asan build, Ubuntu 14.04. Verified fixed FX 49.0, 50.0a2 (2016-09-07), 51.0a1 (2016-09-08) asan builds.
Flags: sec-bounty-
Summary: Heap-buffer-overflow in PropertyProvider::GetSpacingInternal → Out-of-bounds read in PropertyProvider::GetSpacingInternal
Group: core-security-release
Flags: in-testsuite? → in-testsuite+
Flags: sec-bounty-hof+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: