Closed Bug 1437087 Opened 7 years ago Closed 7 years ago

heap-use-after-free in [@ mozilla::EditorEventListener::UninstallFromEditor]

Categories

(Core :: DOM: Editor, defect, P1)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 59+ fixed
firefox58 --- wontfix
firefox59 + fixed
firefox60 + fixed

People

(Reporter: tsmith, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-uaf, sec-high, Whiteboard: [post-critsmash-triage][adv-main59+][adv-esr52.7+])

Attachments

(3 files)

Found with version 20180208-c5120bcaf7bd I am trying to get a reliable testcase but I'm not having much luck. ==7952==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000993980 at pc 0x7f190cbf17c3 bp 0x7ffcf6e6b470 sp 0x7ffcf6e6b468 READ of size 8 at 0x619000993980 thread T0 #0 0x7f190cbf17c2 in mozilla::EditorEventListener::UninstallFromEditor() /src/editor/libeditor/EditorEventListener.cpp:243:49 #1 0x7f190cba7251 in mozilla::EditorEventListener::Disconnect() /src/editor/libeditor/EditorEventListener.cpp:221:3 #2 0x7f190cbedeb4 in mozilla::EditorEventListener::~EditorEventListener() /src/editor/libeditor/EditorEventListener.cpp:115:5 #3 0x7f190cd6d8dd in mozilla::HTMLEditorEventListener::~HTMLEditorEventListener() /src/editor/libeditor/HTMLEditorEventListener.h:24:3 #4 0x7f190cbf1b55 in mozilla::EditorEventListener::Release() /src/editor/libeditor/EditorEventListener.cpp:347:1 #5 0x7f190aeb4264 in UnlinkSelf /src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:545:5 #6 0x7f190aeb4264 in ~CallbackObjectHolder /src/obj-firefox/dist/include/mozilla/dom/CallbackObject.h:411 #7 0x7f190aeb4264 in ~Listener /src/obj-firefox/dist/include/mozilla/EventListenerManager.h:250 #8 0x7f190aeb4264 in Destruct /src/obj-firefox/dist/include/nsTArray.h:541 #9 0x7f190aeb4264 in DestructRange /src/obj-firefox/dist/include/nsTArray.h:2060 #10 0x7f190aeb4264 in ClearAndRetainStorage /src/obj-firefox/dist/include/nsTArray.h:1293 #11 0x7f190aeb4264 in nsTArray_Impl<mozilla::EventListenerManager::Listener, nsTArrayInfallibleAllocator>::Clear() /src/obj-firefox/dist/include/nsTArray.h:1784 #12 0x7f190ae5251e in Clear /src/obj-firefox/dist/include/nsTObserverArray.h:286:12 #13 0x7f190ae5251e in RemoveAllListeners /src/dom/events/EventListenerManager.cpp:173 #14 0x7f190ae5251e in mozilla::EventListenerManager::Disconnect() /src/dom/events/EventListenerManager.cpp:1371 #15 0x7f1908ba40f9 in nsDocument::cycleCollection::Unlink(void*) /src/dom/base/nsDocument.cpp:2089:28 #16 0x7f190b265acd in nsHTMLDocument::cycleCollection::Unlink(void*) /src/dom/html/nsHTMLDocument.cpp:196:1 #17 0x7f19057fd764 in nsCycleCollector::CollectWhite() /src/xpcom/base/nsCycleCollector.cpp:3401:26 #18 0x7f19058003ed in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /src/xpcom/base/nsCycleCollector.cpp:3769:24 #19 0x7f1905804086 in nsCycleCollector_collect(nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:4315:21 #20 0x7f1908cd954b in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /src/dom/base/nsJSEnvironment.cpp:1505:3 #21 0x7f19088209db in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*) /src/dom/base/nsDOMWindowUtils.cpp:1298:3 #22 0x7f190599c841 in NS_InvokeByIndex /src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106 #23 0x7f190742f5d0 in Invoke /src/js/xpconnect/src/XPCWrappedNative.cpp:1948:12 #24 0x7f190742f5d0 in Call /src/js/xpconnect/src/XPCWrappedNative.cpp:1267 #25 0x7f190742f5d0 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /src/js/xpconnect/src/XPCWrappedNative.cpp:1234 #26 0x7f1907435e33 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:929:12 #27 0x7f191145ffc4 in CallJSNative /src/js/src/jscntxtinlines.h:291:15 #28 0x7f191145ffc4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473 #29 0x7f1911449c34 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12 #30 0x7f1911449c34 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096 #31 0x7f191142be37 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12 #32 0x7f1911460181 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15 #33 0x7f1911460e23 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10 #34 0x7f1912068025 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2978:12 #35 0x7f1907346f70 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/ExportHelpers.cpp:315:18 #36 0x7f191145ffc4 in CallJSNative /src/js/src/jscntxtinlines.h:291:15 #37 0x7f191145ffc4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473 #38 0x7f1911449c34 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12 #39 0x7f1911449c34 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096 #40 0x7f191142be37 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12 #41 0x7f1911460181 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15 #42 0x7f1911460e23 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10 #43 0x7f191206a99f in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3037:12 #44 0x7f190a2e77ef in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8 #45 0x7f190ae5ced3 in HandleEvent<mozilla::dom::EventTarget *> /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12 #46 0x7f190ae5ced3 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1108 #47 0x7f190ae5ee26 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1286:20 #48 0x7f190ae48642 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:490:16 #49 0x7f190ae4c073 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:859:9 #50 0x7f190ae4deac in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) /src/dom/events/EventDispatcher.cpp:926:12 #51 0x7f1908cbe56f in nsINode::DispatchEvent(nsIDOMEvent*, bool*) /src/dom/base/nsINode.cpp:1275:5 #52 0x7f19087d066e in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsTSubstring<char16_t> const&, bool, bool, bool, bool*, bool) /src/dom/base/nsContentUtils.cpp:4534:18 #53 0x7f19087d0424 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsTSubstring<char16_t> const&, bool, bool, bool*) /src/dom/base/nsContentUtils.cpp:4502:10 #54 0x7f1908bce5e1 in nsDocument::DispatchContentLoadedEvents() /src/dom/base/nsDocument.cpp:5376:3 #55 0x7f1908c342f4 in applyImpl<nsDocument, void (nsDocument::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1149:12 #56 0x7f1908c342f4 in apply<nsDocument, void (nsDocument::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1155 #57 0x7f1908c342f4 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1200 #58 0x7f19059700db in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1040:14 #59 0x7f190598c1e0 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:517:10 #60 0x7f19068229aa in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21 #61 0x7f1906773979 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10 #62 0x7f1906773979 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319 #63 0x7f1906773979 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299 #64 0x7f190ca6821a in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27 #65 0x7f1910f294bb in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30 #66 0x7f19111367f5 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4676:22 #67 0x7f191113976c in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4811:8 #68 0x7f191113abb4 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4903:21 #69 0x4f6d45 in do_main /src/browser/app/nsBrowserApp.cpp:231:22 #70 0x4f6d45 in main /src/browser/app/nsBrowserApp.cpp:304 #71 0x7f19247c782f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291 #72 0x4265bc in _start (/home/ubuntu/firefox/firefox+0x4265bc) 0x619000993980 is located 0 bytes inside of 952-byte region [0x619000993980,0x619000993d38) freed by thread T0 here: #0 0x4c6fc2 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:68:3 #1 0x7f19057f9210 in SnowWhiteKiller::~SnowWhiteKiller() /src/xpcom/base/nsCycleCollector.cpp:2729:25 #2 0x7f1905800def in FreeSnowWhite /src/xpcom/base/nsCycleCollector.cpp:2917:3 #3 0x7f1905800def in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:3925 #4 0x7f19058003b4 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /src/xpcom/base/nsCycleCollector.cpp:3746:9 #5 0x7f1905804086 in nsCycleCollector_collect(nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:4315:21 #6 0x7f1908cd954b in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /src/dom/base/nsJSEnvironment.cpp:1505:3 #7 0x7f19088209db in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*) /src/dom/base/nsDOMWindowUtils.cpp:1298:3 #8 0x7f190599c841 in NS_InvokeByIndex /src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106 #9 0x7f190742f5d0 in Invoke /src/js/xpconnect/src/XPCWrappedNative.cpp:1948:12 #10 0x7f190742f5d0 in Call /src/js/xpconnect/src/XPCWrappedNative.cpp:1267 #11 0x7f190742f5d0 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /src/js/xpconnect/src/XPCWrappedNative.cpp:1234 #12 0x7f1907435e33 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:929:12 #13 0x7f191145ffc4 in CallJSNative /src/js/src/jscntxtinlines.h:291:15 #14 0x7f191145ffc4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473 #15 0x7f1911449c34 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12 #16 0x7f1911449c34 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096 #17 0x7f191142be37 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12 #18 0x7f1911460181 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15 #19 0x7f1911460e23 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10 #20 0x7f1912068025 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2978:12 #21 0x7f1907346f70 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/ExportHelpers.cpp:315:18 #22 0x7f191145ffc4 in CallJSNative /src/js/src/jscntxtinlines.h:291:15 #23 0x7f191145ffc4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473 #24 0x7f1911449c34 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12 #25 0x7f1911449c34 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096 #26 0x7f191142be37 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12 #27 0x7f1911460181 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15 #28 0x7f1911460e23 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10 #29 0x7f191206a99f in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3037:12 #30 0x7f190a2e77ef in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8 #31 0x7f190ae5ced3 in HandleEvent<mozilla::dom::EventTarget *> /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12 #32 0x7f190ae5ced3 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1108 #33 0x7f190ae5ee26 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1286:20 #34 0x7f190ae48642 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:490:16 #35 0x7f190ae4c073 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:859:9 #36 0x7f190ae4deac in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) /src/dom/events/EventDispatcher.cpp:926:12 #37 0x7f1908cbe56f in nsINode::DispatchEvent(nsIDOMEvent*, bool*) /src/dom/base/nsINode.cpp:1275:5 previously allocated by thread T0 here: #0 0x4c7303 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3 #1 0x4f7dcd in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:70:17 #2 0x7f190cdf6f39 in operator new /src/obj-firefox/dist/include/mozilla/mozalloc.h:159:12 #3 0x7f190cdf6f39 in nsEditingSession::SetupEditorOnWindow(mozIDOMWindowProxy*) /src/editor/composer/nsEditingSession.cpp:424 #4 0x7f190cdf435f in nsEditingSession::MakeWindowEditable(mozIDOMWindowProxy*, char const*, bool, bool, bool) /src/editor/composer/nsEditingSession.cpp:178:10 #5 0x7f190b26ec36 in nsHTMLDocument::EditingStateChanged() /src/dom/html/nsHTMLDocument.cpp:2618:25 #6 0x7f190b27d6be in nsHTMLDocument::MaybeEditingStateChanged() /src/dom/html/nsHTMLDocument.cpp:2259:7 #7 0x7f1908b35d87 in nsContentSink::StartLayout(bool) /src/dom/base/nsContentSink.cpp:1259:14 #8 0x7f1907aeaa85 in nsHtml5TreeOpExecutor::StartLayout(bool*) /src/parser/html/nsHtml5TreeOpExecutor.cpp:673:18 #9 0x7f1907ae595e in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*) /src/parser/html/nsHtml5TreeOperation.cpp:1215:17 #10 0x7f1907ae2d26 in nsHtml5TreeOpExecutor::RunFlushLoop() /src/parser/html/nsHtml5TreeOpExecutor.cpp:493:29 #11 0x7f1907af024f in nsHtml5ExecutorReflusher::Run() /src/parser/html/nsHtml5TreeOpExecutor.cpp:57:18 #12 0x7f19059700db in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1040:14 #13 0x7f190598c1e0 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:517:10 #14 0x7f19068229aa in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21 #15 0x7f1906773979 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10 #16 0x7f1906773979 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319 #17 0x7f1906773979 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299 #18 0x7f190ca6821a in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27 #19 0x7f1910f294bb in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30 #20 0x7f19111367f5 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4676:22 #21 0x7f191113976c in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4811:8 #22 0x7f191113abb4 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4903:21 #23 0x4f6d45 in do_main /src/browser/app/nsBrowserApp.cpp:231:22 #24 0x4f6d45 in main /src/browser/app/nsBrowserApp.cpp:304 #25 0x7f19247c782f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
hmm hmm, mEditorBase is a raw pointer.
Assignee: nobody → m_kato
Keywords: sec-high
Group: dom-core-security → layout-core-security
EditorEventListener::Disconnect isn't called because editor is destroyed (unlinked) by cycle collector?
Priority: -- → P1
Comment on attachment 8951536 [details] [diff] [review] Call Disconnect on Unlink of cycle collector I am not sure how to reproduce this. But when EditorBase is destroyed by cycle collector, we mightn't call EditorEventListener::Disconnect. So we have to call it on Unlink. Also, we can store EditorEventListener directly instead of nsIDOMEventListener. So we should use it to avoid reinterpret_cast.
Attachment #8951536 - Flags: review?(masayuki)
Attachment #8951536 - Flags: review?(masayuki) → review+
Comment on attachment 8951536 [details] [diff] [review] Call Disconnect on Unlink of cycle collector [Security approval request comment] How easily could an exploit be constructed based on the patch? It is difficult to create exploit even if this is a kind of use-after-free. Because this depends on cycle collector. When cycle collector destroys editor (EditorBase), we don't disconnect event listener correctly, then UAF might occur. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? From patch, this depends on cycle collector. But attacker cannot imagine creating exploit since web page cannot control cycle collector. Which older supported branches are affected by this flaw? At least, this occurs on ESR52. If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Yes, it might require rebase, but it is simple. How likely is this patch to cause regressions; how much testing does it need? Too low. It depends on cycle collector, so it is difficult to confirm this fix on manual test.
Attachment #8951536 - Flags: sec-approval?
Attached patch for ESR52Splinter Review
Attachment #8952074 - Attachment description: for old branch → for ESR52
Attached patch For betaSplinter Review
Comment on attachment 8952076 [details] [diff] [review] For beta Approval Request Comment [Feature/Bug causing the regression]: Nothing [User impact if declined]: UAF by cycle collector. [Is this code covered by automated tests?]: No. I am not sure how to reproduce this [Has the fix been verified in Nightly?]: No. I am not sure how to reproduce this. Also this is a kind of UAF. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: Nothing. [Is the change risky?]: Low. [Why is the change risky/not risky?]: Reset dependency of objects correctly when on cycle collector. [String changes made/needed]: No
Attachment #8952076 - Flags: approval-mozilla-beta?
Comment on attachment 8952074 [details] [diff] [review] for ESR52 [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is sec-high User impact if declined: UAF by cycle collector Fix Landed on Version: 60 Risk to taking this patch (and alternatives if risky): Low. Reset dependency of objects correctly when on cycle collector. String or UUID changes made by this patch: No. See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8952074 - Flags: approval-mozilla-esr52?
Tracking to make sure we get these landed if they get sec approval. Wontfix for 58 at this point since we are close to 59 release.
sec-approval+ for trunk and I'll give beta approval. I think we need this on ESR52 as well.
Attachment #8951536 - Flags: sec-approval? → sec-approval+
Attachment #8952076 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 8952074 [details] [diff] [review] for ESR52 Approved for ESR 52.7.0.
Attachment #8952074 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Group: layout-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main59+][adv-esr52.7+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: