Closed Bug 1452627 Opened 5 years ago Closed 5 years ago

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:78:9 in ~RefPtr

Categories

(Core :: CSS Parsing and Computation, defect, P1)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + fixed

People

(Reporter: jkratzer, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, regression, testcase)

Attachments

(2 files, 2 obsolete files)

trigger.html
437 bytes, text/html
Details
Updated patch.
4.56 KB, patch
bholley
: review+
Details | Diff | Splinter Review 
Testcase found while fuzzing mozilla-central rev 30d72755b174.

I'm currently reducing the testcase and will update once complete.

==9194==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000303578 at pc 0x7f195ba91ec3 bp 0x7ffca61f7990 sp 0x7ffca61f7988
READ of size 8 at 0x602000303578 thread T0 (file:// Content)
    #0 0x7f195ba91ec2 in ~RefPtr /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:78:9
    #1 0x7f195ba91ec2 in ~nsStyleContentAttr /builds/worker/workspace/build/src/obj-firefox/dist/include/nsStyleStruct.h:2619
    #2 0x7f195ba91ec2 in nsStyleContentData::~nsStyleContentData() /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:4149
    #3 0x7f195ba92577 in Destruct /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:542:45
    #4 0x7f195ba92577 in DestructRange /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:2073
    #5 0x7f195ba92577 in ClearAndRetainStorage /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:1294
    #6 0x7f195ba92577 in ~nsTArray_Impl /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:865
    #7 0x7f195ba92577 in nsStyleContent::~nsStyleContent() /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:4247
    #8 0x7f19619e9771 in style::gecko_properties::_$LT$impl$u20$core..ops..drop..Drop$u20$for$u20$style..gecko_bindings..structs..root..mozilla..GeckoContent$GT$::drop::hf813265729a60f5d /builds/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-8256faa5219835bc/out/gecko_properties.rs:23311
    #9 0x7f19619e9771 in core::ptr::drop_in_place::h93c82ca0160f8a8f /checkout/src/libcore/ptr.rs:59
    #10 0x7f19619e9771 in core::ptr::drop_in_place::ha7c732fd695046aa /checkout/src/libcore/ptr.rs:59
    #11 0x7f19619e9771 in core::ptr::drop_in_place::h901ca99a726e8d89 /checkout/src/libcore/ptr.rs:59
    #12 0x7f19619e9771 in _$LT$servo_arc..Arc$LT$T$GT$$GT$::drop_slow::h2ee3959dd674c95f /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:256
    #13 0x7f19619e945c in _$LT$servo_arc..Arc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::ha872011e46f724c8 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:390
    #14 0x7f19619e945c in core::ptr::drop_in_place::h8b0659367bfc0f17 /checkout/src/libcore/ptr.rs:59
    #15 0x7f19619e945c in _$LT$servo_arc..RawOffsetArc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::hb676e2fb4c74b515 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:796
    #16 0x7f19619e945c in core::ptr::drop_in_place::ha9701c1686ce36b5 /checkout/src/libcore/ptr.rs:59
    #17 0x7f19619e945c in core::ptr::drop_in_place::hd0165e7a974f3166 /checkout/src/libcore/ptr.rs:59
    #18 0x7f19619e945c in core::ptr::drop_in_place::h744d8b331309418d /checkout/src/libcore/ptr.rs:59
    #19 0x7f19619e945c in core::ptr::drop_in_place::h0b746fb5a2e17d1a /checkout/src/libcore/ptr.rs:59
    #20 0x7f19619e945c in core::ptr::drop_in_place::h62feb9309f63304f /checkout/src/libcore/ptr.rs:59
    #21 0x7f19619e945c in core::ptr::drop_in_place::h73e2f5b554047fb6 /checkout/src/libcore/ptr.rs:59
    #22 0x7f19619e945c in _$LT$servo_arc..Arc$LT$T$GT$$GT$::drop_slow::h0dff2844740cb23b /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:256
    #23 0x7f19619f20aa in _$LT$servo_arc..Arc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::hb71afb9e233fa3e6 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:390
    #24 0x7f19619f20aa in core::ptr::drop_in_place::h96edd122bc65adb7 /checkout/src/libcore/ptr.rs:59
    #25 0x7f19619f20aa in style::gecko::arc_types::Servo_ComputedStyle_Release::_$u7b$$u7b$closure$u7d$$u7d$::hf1ae763e4bb946dd /builds/worker/workspace/build/src/servo/components/style/gecko/arc_types.rs:140
    #26 0x7f19619f20aa in _$LT$servo_arc..ArcBorrow$LT$$u27$a$C$$u20$T$GT$$GT$::with_arc::h11df6961bd8ccde7 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:939
    #27 0x7f19619f20aa in Servo_ComputedStyle_Release /builds/worker/workspace/build/src/servo/components/style/gecko/arc_types.rs:139
    #28 0x7f195c2545c7 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ComputedStyle.h:131:20
    #29 0x7f195c2545c7 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:41
    #30 0x7f195c2545c7 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:398
    #31 0x7f195c2545c7 in ~RefPtr /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:79
    #32 0x7f195c2545c7 in ~nsIFrame /builds/worker/workspace/build/src/layout/generic/nsIFrame.h:574
    #33 0x7f195c2545c7 in nsBox::~nsBox() /builds/worker/workspace/build/src/layout/xul/nsBox.cpp:194
    #34 0x7f195bdb3777 in nsFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:816:9
    #35 0x7f195bd623a8 in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:303:22
    #36 0x7f195bf6869c in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsLineBox.cpp:402:14
    #37 0x7f195bd617c2 in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsBlockFrame.cpp:327:3
    #38 0x7f195bd622fd in DestroyFramesFrom /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:59:12
    #39 0x7f195bd622fd in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:230
    #40 0x7f195bdc8b35 in nsCanvasFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsCanvasFrame.cpp:167:21
    #41 0x7f195bd622fd in DestroyFramesFrom /builds/worker/workspace/build/src/layout/generic/nsFrameList.cpp:59:12
    #42 0x7f195bd622fd in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:230
    #43 0x7f195bddf070 in Destroy /builds/worker/workspace/build/src/layout/generic/nsIFrame.h:689:5
    #44 0x7f195bddf070 in nsContainerFrame::RemoveFrame(mozilla::layout::FrameChildListID, nsIFrame*) /builds/worker/workspace/build/src/layout/generic/nsContainerFrame.cpp:179
    #45 0x7f195bbf84aa in RemoveFrame /builds/worker/workspace/build/src/layout/base/nsFrameManager.cpp:129:18
    #46 0x7f195bbf84aa in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:8161
    #47 0x7f195bbe634c in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:9209:5
    #48 0x7f195bb7f0b0 in mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:1509:25
    #49 0x7f195bb8b945 in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:2997:9
    #50 0x7f195bb4433d in ProcessPendingRestyles /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:3073:3
    #51 0x7f195bb4433d in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4282
    #52 0x7f1956c546a8 in FlushPendingNotifications /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIPresShell.h:592:5
    #53 0x7f1956c546a8 in nsIDocument::FlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:7574
    #54 0x7f195bb3ccd5 in mozilla::PresShell::ScrollContentIntoView(nsIContent*, nsIPresShell::ScrollAxis, nsIPresShell::ScrollAxis, unsigned int) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:3577:16
    #55 0x7f1956a3bd60 in mozilla::dom::Element::ScrollIntoView(mozilla::dom::ScrollIntoViewOptions const&) /builds/worker/workspace/build/src/dom/base/Element.cpp:810:14
    #56 0x7f1956a3ba25 in mozilla::dom::Element::ScrollIntoView(mozilla::dom::BooleanOrScrollIntoViewOptions const&) /builds/worker/workspace/build/src/dom/base/Element.cpp:750:10
    #57 0x7f1958aa30a7 in mozilla::dom::ElementBinding::scrollIntoView(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:2878:9
    #58 0x7f195921d631 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3191:13
    #59 0x7f195fb32927 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/JSContext-inl.h:290:15
    #60 0x7f195fb32927 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:467
    #61 0x7f195fb1d3e1 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:522:12
    #62 0x7f195fb1d3e1 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3084
    #63 0x7f195fb0382a in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:417:12
    #64 0x7f195fb326a5 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:489:15
    #65 0x7f195fb33922 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:535:10
    #66 0x7f196065484d in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3003:12
    #67 0x7f19589669df in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/EventListenerBinding.cpp:51:8
    #68 0x7f19599523f1 in HandleEvent<mozilla::dom::EventTarget *> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
    #69 0x7f19599523f1 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1104
    #70 0x7f1959953cb5 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1276:20
    #71 0x7f195993e047 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:527:16
    #72 0x7f1959941de7 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:914:9
    #73 0x7f195bc25928 in nsDocumentViewer::LoadComplete(nsresult) /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp:1066:7
    #74 0x7f195edd7ddb in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7285:21
    #75 0x7f195edd3f49 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:7078:7
    #76 0x7f195eddbadf in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp
    #77 0x7f195587ec87 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:1315:3
    #78 0x7f195587dd0a in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:858:14
    #79 0x7f195587a8e5 in nsDocLoader::DocLoaderIsEmpty(bool) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:747:9
    #80 0x7f195587c8ac in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:632:5
    #81 0x7f195587d8cc in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp
    #82 0x7f1953c0194a in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/netwerk/base/nsLoadGroup.cpp:629:28
    #83 0x7f1956c5a61a in DoUnblockOnload /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:8409:18
    #84 0x7f1956c5a61a in nsDocument::UnblockOnload(bool) /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:8331
    #85 0x7f1956c3a51a in nsIDocument::DispatchContentLoadedEvents() /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:5314:3
    #86 0x7f1956d50a74 in applyImpl<nsIDocument, void (nsIDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1164:12
    #87 0x7f1956d50a74 in apply<nsIDocument, void (nsIDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1170
    #88 0x7f1956d50a74 in mozilla::detail::RunnableMethodImpl<nsIDocument*, void (nsIDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1215
    #89 0x7f1953a0a5c1 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:337:32
    #90 0x7f1953a29f18 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1096:14
    #91 0x7f1953a46350 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:519:10
    #92 0x7f195491a45a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #93 0x7f195486a229 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #94 0x7f195486a229 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #95 0x7f195486a229 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #96 0x7f195b58beba in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27
    #97 0x7f195f84dedb in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:893:22
    #98 0x7f195486a229 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #99 0x7f195486a229 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #100 0x7f195486a229 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #101 0x7f195f84d8a2 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:719:34
    #102 0x4f50dc in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #103 0x4f50dc in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280
    #104 0x7f197390b82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #105 0x42476c in _start (/home/forb1dden/builds/mc-asan/firefox+0x42476c)

0x602000303578 is located 0 bytes to the right of 8-byte region [0x602000303570,0x602000303578)
allocated by thread T0 (file:// Content) here:
    #0 0x4c54b3 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
    #1 0x4f5f7d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:70:17
    #2 0x7f195389bc1e in NS_strndup<char16_t> /builds/worker/workspace/build/src/xpcom/base/nsCRTGlue.cpp:134:25
    #3 0x7f195389bc1e in NS_strdup(char16_t const*) /builds/worker/workspace/build/src/xpcom/base/nsCRTGlue.cpp:127
    #4 0x7f195ba91ff9 in nsStyleContentData /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:4167:24
    #5 0x7f195ba91ff9 in nsStyleContentData::operator=(nsStyleContentData const&) /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:4189
    #6 0x7f195b99c6ee in Gecko_CopyStyleContentsFrom /builds/worker/workspace/build/src/layout/style/ServoBindings.cpp:1740:28
    #7 0x7f196189cda8 in style::properties::apply_declarations::h8da774778ccd7060 /builds/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-8256faa5219835bc/out/properties.rs:136435
    #8 0x7f196189cda8 in style::properties::cascade::hf25791bb08df8d60 /builds/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-8256faa5219835bc/out/properties.rs:136038
    #9 0x7f196189b5fd in style::stylist::Stylist::cascade_style_and_visited::h7d6c47becf65635e /builds/worker/workspace/build/src/servo/components/style/stylist.rs:921
    #10 0x7f196189ac8c in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::cascade_style_and_visited::hf3e35cf9983a91ea /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:304
    #11 0x7f1961899e42 in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::cascade_primary_style::h8dd91823f8f03e53 /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:213
    #12 0x7f196189861e in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::resolve_primary_style::h06fb4ee626d175d1 /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:172
    #13 0x7f196189861e in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::resolve_style::h4038fb8122599bb3 /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:230
    #14 0x7f196188546e in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::resolve_style_with_default_parents::_$u7b$$u7b$closure$u7d$$u7d$::h4c06e001f07c4423 /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:268
    #15 0x7f196188546e in style::style_resolver::with_default_parent_styles::hc396cdcde64d72f4 /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:102
    #16 0x7f196188546e in _$LT$style..style_resolver..StyleResolverForElement$LT$$u27$a$C$$u20$$u27$ctx$C$$u20$$u27$le$C$$u20$E$GT$$GT$::resolve_style_with_default_parents::h846a12a6cdfadbf1 /builds/worker/workspace/build/src/servo/components/style/style_resolver.rs:267
    #17 0x7f196188546e in style::traversal::compute_style::h147f579ebedd31b6 /builds/worker/workspace/build/src/servo/components/style/traversal.rs:611
    #18 0x7f196188287b in style::traversal::recalc_style_at::h245e2322c5a1999f /builds/worker/workspace/build/src/servo/components/style/traversal.rs:420
    #19 0x7f196188287b in _$LT$style..gecko..traversal..RecalcStyleOnly$LT$$u27$recalc$GT$$u20$as$u20$style..traversal..DomTraversal$LT$style..gecko..wrapper..GeckoElement$LT$$u27$le$GT$$GT$$GT$::process_preorder::h6ddeb6e669b77364 /builds/worker/workspace/build/src/servo/components/style/gecko/traversal.rs:37
    #20 0x7f196188287b in style::driver::traverse_dom::hdf5edcec64a539f6 /builds/worker/workspace/build/src/servo/components/style/driver.rs:107
    #21 0x7f196188287b in geckoservo::glue::traverse_subtree::hb8c3e84487475f3c /builds/worker/workspace/build/src/servo/ports/geckolib/glue.rs:289
    #22 0x7f1961882097 in Servo_TraverseSubtree /builds/worker/workspace/build/src/servo/ports/geckolib/glue.rs:347
    #23 0x7f195b9d6837 in mozilla::ServoStyleSet::StyleDocument(mozilla::ServoTraversalFlags) /builds/worker/workspace/build/src/layout/style/ServoStyleSet.cpp:1016:7
    #24 0x7f195bb8b083 in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:2965:20
    #25 0x7f195bb4433d in ProcessPendingRestyles /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:3073:3
    #26 0x7f195bb4433d in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4282
    #27 0x7f1956c546a8 in FlushPendingNotifications /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIPresShell.h:592:5
    #28 0x7f1956c546a8 in nsIDocument::FlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:7574
    #29 0x7f195b9e74a2 in nsComputedDOMStyle::GetComputedStyle(mozilla::dom::Element*, nsAtom*, nsComputedDOMStyle::StyleType) /builds/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:461:10
    #30 0x7f19567de628 in GetTargetComputedStyle /builds/worker/workspace/build/src/dom/animation/KeyframeEffectReadOnly.cpp:836:10
    #31 0x7f19567de628 in mozilla::dom::KeyframeEffectReadOnly::SetKeyframes(JSContext*, JS::Handle<JSObject*>, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/animation/KeyframeEffectReadOnly.cpp:193
    #32 0x7f19567d82a7 in already_AddRefed<mozilla::dom::KeyframeEffect> mozilla::dom::KeyframeEffectReadOnly::ConstructKeyframeEffect<mozilla::dom::KeyframeEffect, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions>(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/animation/KeyframeEffectReadOnly.cpp:668:11
    #33 0x7f1956a55772 in mozilla::dom::Element::Animate(mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JSContext*, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/base/Element.cpp:3692:5
    #34 0x7f1956a55262 in mozilla::dom::Element::Animate(JSContext*, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/base/Element.cpp:3657:10
    #35 0x7f1958aa9410 in mozilla::dom::ElementBinding::animate(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:4289:61
    #36 0x7f195921d631 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3191:13
    #37 0x7f195fb32927 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/JSContext-inl.h:290:15
    #38 0x7f195fb32927 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:467
    #39 0x7f195fb1d3e1 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:522:12
    #40 0x7f195fb1d3e1 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3084
    #41 0x7f195fb0382a in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:417:12
    #42 0x7f195fb326a5 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:489:15
    #43 0x7f195fb33922 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:535:10
    #44 0x7f196065484d in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3003:12

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:78:9 in ~RefPtr
Shadow bytes around the buggy address:
  0x0c0480058650: fa fa 02 fa fa fa 01 fa fa fa 01 fa fa fa 01 fa
  0x0c0480058660: fa fa 00 03 fa fa 02 fa fa fa 01 fa fa fa 01 fa
  0x0c0480058670: fa fa 01 fa fa fa 07 fa fa fa 05 fa fa fa 00 00
  0x0c0480058680: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c0480058690: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
=>0x0c04800586a0: fa fa fd fd fa fa fd fd fa fa 00 00 fa fa 00[fa]
  0x0c04800586b0: fa fa 00 00 fa fa 00 00 fa fa fd fa fa fa fd fd
  0x0c04800586c0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c04800586d0: fa fa fd fd fa fa 00 00 fa fa 00 00 fa fa fd fd
  0x0c04800586e0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c04800586f0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==9194==ABORTING

[Dumping log 'log_ffp_worker_log_scanner.txt' (0.10KB)]
TOKEN_LOCATED: ###!!! [Parent][MessageChannel] Error: (msgtype=0x16007F,name=PBrowser::Msg_Destroy)

[Dumping log 'log_stdout.txt' (0.17KB)]
DOMFuzzHelper created
DOMFuzzHelper created
DOMFuzzHelper created
DOMFuzzHelper created
DOMFuzzHelper created
DOMFuzzHelper created
DOMFuzzHelper created
DOMFuzzHelper created

[Dumping log 'log_stderr.txt' (0.75KB)]
[ffpuppet] Launch command: /home/forb1dden/builds/mc-asan/firefox -no-remote -profile /tmp/ffprof_P4quQi http://127.0.0.1:11312

ATTENTION: default value of option force_s3tc_enable overridden by environment.
ATTENTION: default value of option force_s3tc_enable overridden by environment.
JavaScript error: jar:file:///home/forb1dden/builds/mc-asan/omni.ja!/components/captivedetect.js, line 236: NS_ERROR_FAILURE: No canonical URL set up.
[Parent 9072, Gecko_IOThread] WARNING: pipe error (85): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353

###!!! [Parent][MessageChannel] Error: (msgtype=0x16007F,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv

[ffpuppet] Exit code: -15
Attached file trigger.html — 

    
  
Keywords: testcase-wanted → testcase

    
    

    
  
Emilio: any ideas? This is destroying something allocated by Stylo, maybe reading atoms that aren't there anymore? Or just plain garbage pointers?
Flags: needinfo?(emilio)

    
    

    
  
To me this looked like a UAF that didn't fit in ASAN's redzone, attempting with the appropriate ASAN_OPTIONS might improve the stacktrace. This is based on the alleged allocation being a strdup, but the overread being in ~RefPtr.

    
    

    
  
This is likely related to Bobby's recent changes to namespace registration... I can take a closer look tomorrow probably.
Blocks: 1451421

    
    

    
  
Ok I know why this is. Argh, c++
Assignee: nobody → emilio
Flags: needinfo?(emilio)

    
    

    
  


  
This is trunk-only. This is a good argument for the "use rust types in style structs" thing, just saying :P

        Attachment #8967485 -
        Flags: review?(bobbyholley)

    
    

    
  
I've seen bugmail about a pretty similar bug. Cam, Nick, do you remember which, it's likely a dupe of this one.
Flags: needinfo?(n.nethercote)
Flags: needinfo?(cam)

    
    

    
  
Oh, boy, it's still worse.

    
    

    
  
Comment on attachment 8967490 [details] [diff] [review]
Fix a couple other memory safety issues from bug 1451421.

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

::: layout/style/nsStyleStruct.cpp
@@ +4141,5 @@
>  {
>    MOZ_COUNT_DTOR(nsStyleContentData);
>  
>    if (mType == eStyleContentType_Image) {
> +    // FIXME(emilio): Is this needed now that URLs are main thread only?

This should say _are not_ main-thread only.

    
    

    
  
Comment on attachment 8967485 [details] [diff] [review]
Fix nsStyleContent copy constructor.

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

r=me, but the old code would have just set the union pointer to zero, which wouldn't have caused a UAF, right?

        Attachment #8967485 -
        Flags: review?(bobbyholley) → review+

    
    

    
  
Comment on attachment 8967490 [details] [diff] [review]
Fix a couple other memory safety issues from bug 1451421.

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

Same here - none of these seem like memory hazards to me.

        Attachment #8967490 -
        Flags: review?(bobbyholley) → review+

    
    

    
  
NI emilio on why these aren't just safe null derefs.
Flags: needinfo?(emilio)

    
    

    
  
How is it a safe null deref? It's a union.

For (1) it's confusing an nsStyleContentAttr* with a uint16_t* and calling strdup on it, copying whatever garbage there is until a null is hit, and forgetting to addref the nsAtom*. It's just a UAF because in practice, since there's no namespace pointer (it's null), you effectively copy the atom pointer without addreffing it, which is what causes the UAF on destruction. But it'd be a heap-buffer-overflow (on top of that) if it actually was using attr(foo|bar).

Same for (2), it's calling strcmp in a nsStyleContentAttr*, which makes no sense at all. I guess that again it's not a heap overflow because the fuzzer didn't use the namespace ID and thus it was null but it'd be a heap-buffer-overflow with a namespace id similarly.

Does that make sense?
Flags: needinfo?(emilio) → needinfo?(bobbyholley)

    
    

    
  
(In reply to Emilio Cobos Álvarez [:emilio] from comment #14)
> How is it a safe null deref? It's a union.
> 
> For (1) it's confusing an nsStyleContentAttr* with a uint16_t* and calling
> strdup on it, copying whatever garbage there is until a null is hit, and
> forgetting to addref the nsAtom*. It's just a UAF because in practice, since
> there's no namespace pointer (it's null), you effectively copy the atom
> pointer without addreffing it, which is what causes the UAF on destruction.
> But it'd be a heap-buffer-overflow (on top of that) if it actually was using
> attr(foo|bar).

Oh, the thing that I missed was that the case is:

> else if (aOther.mContent.mString) {

rather than

> else if (mType == eStyleContentType_String) {

The latter would mean that we'd just end up with

> mContent.mString = nullptr

which would be fine.

To avoid this confusion in the future, I think it would be nice to make all if the conditional branches use mType. Please add that to the patch if you're willing, though I'm ok with landing as-is if you're in a hurry.

> Same for (2), it's calling strcmp in a nsStyleContentAttr*, which makes no
> sense at all. I guess that again it's not a heap overflow because the fuzzer
> didn't use the namespace ID and thus it was null but it'd be a
> heap-buffer-overflow with a namespace id similarly.

Yeah, I don't know what I was thinking on this one. A similar change to the conditional would be appreciated.
Flags: needinfo?(bobbyholley)

    
    

    
  
Comment on attachment 8967485 [details] [diff] [review]
Fix nsStyleContent copy constructor.

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

::: layout/style/nsStyleStruct.cpp
@@ +4173,2 @@
>    } else if (aOther.mContent.mString) {
> +    MOZ_ASSERT(mType == eStyleContentType_String);

I know nothing about this code, but it seems weird and error-prone to check mString before mType.

Can it be rewritten so (a) all the conditions are on mType, (b) we assert if mType is not recognized, and (c) mString is only accessed once mType is known to be eStyleContentType_String?

    
    

    
  
> I've seen bugmail about a pretty similar bug. Cam, Nick, do you remember
> which, it's likely a dupe of this one.

Sorry, I have no memory of anything like this.
Flags: needinfo?(n.nethercote)

    
    

    
  
(In reply to Nicholas Nethercote [:njn] from comment #16)
> Can it be rewritten so (a) all the conditions are on mType, (b) we assert if
> mType is not recognized, and (c) mString is only accessed once mType is
> known to be eStyleContentType_String?

Hah, this is basically what I asked for in comment 15. :-)

    
    

    
  
(In reply to Nicholas Nethercote [:njn] from comment #17)
> > I've seen bugmail about a pretty similar bug. Cam, Nick, do you remember
> > which, it's likely a dupe of this one.
> 
> Sorry, I have no memory of anything like this.

I meant bug 1452478.
Flags: needinfo?(cam)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Updated patch.
        — 
          — Splinter Review
      

    


  
I forgot about all the other quote types and what not so my patch asserted. I tweaked the stuff to make sure that we don't stash more pointers without noticing, but enumerating all the types was somewhat verbose.

Bobby, you fine with this?

        Attachment #8967485 -
        Attachment is obsolete: true

        Attachment #8967490 -
        Attachment is obsolete: true

        Attachment #8967541 -
        Flags: review?(bobbyholley)

    
    

    
  
(In reply to Emilio Cobos Álvarez [:emilio] from comment #20)
> Created attachment 8967541 [details] [diff] [review]
> Updated patch.
> 
> I forgot about all the other quote types and what not so my patch asserted.
> I tweaked the stuff to make sure that we don't stash more pointers without
> noticing, but enumerating all the types was somewhat verbose.
> 
> Bobby, you fine with this?

Actually no need to null-check the mString pointer if the type is String. So add that to the patch.

    
    

    
  
Comment on attachment 8967541 [details] [diff] [review]
Updated patch.

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

::: layout/style/nsStyleStruct.cpp
@@ +4161,5 @@
>  nsStyleContentData::nsStyleContentData(const nsStyleContentData& aOther)
>    : mType(aOther.mType)
>  {
>    MOZ_COUNT_CTOR(nsStyleContentData);
> +  switch (mType) {

I think an if/else on mType would be moderately more consistent with surrounding code than a swtich, but don't feel strongly.

        Attachment #8967541 -
        Flags: review?(bobbyholley) → review+

    
    

    
  
(In reply to Bobby Holley (:bholley) from comment #22
> I think an if/else on mType would be moderately more consistent with
> surrounding code than a swtich, but don't feel strongly.

I didn't bother since Resolve(..) also used a switch.
Version: 59 Branch → Trunk

    
    

    
  
(In reply to Emilio Cobos Álvarez [:emilio] from comment #19)
> I meant bug 1452478.

Yes, I do remember seeing some similar stacks in that bug.  But a bunch aren't, so I assume there is another source of UAFs manifesting in touching points that are no longer valid nsAtoms somewhere.
Priority: -- → P1

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/2c051e8bfb3e
Status: NEW → RESOLVED
Closed: 5 years ago

          status-firefox61:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Group: core-security → core-security-release

    
  
Depends on: 1457985
Group: core-security-release

          You need to log in
          before you can comment on or make changes to this bug.