Crash found while fuzzing mc-asan-debug rev 20170807-47248637eafa. Due to the unreliability of the testcase, minimization was not possible. Furthermore, the attached testcase includes (and requires) the full fuzzIDL database - likely due to memory pressure. Please do not open this bug once it's been resolved as it may expose us to unknown risks. Testcase may require multiple iterations in order to trigger the issue. Steps to reproduce: 1. Install ffpuppet (https://github.com/MozillaSecurity/ffpuppet) 2. Download the fuzzpriv extension (https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension) 2. Unpack testcase 3. Start local webserver in testcase directory (python -m SimpleHTTPServer) 4. From the testcase directory, launch testcase with ffpuppet until crash occurs: while true; do python /path/to/ffpuppet/ffpuppet.py /path/to/firefox/firefox -e /path/to/fuzzpriv/extension/ -p prefs.js -l log.txt -u http://localhost:8000/harness.html --xvfb; if grep -q 'AddressSanitizer' log.txt then break; fi done ==25979==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000096810 at pc 0x7f491e356ac5 bp 0x7ffddb44c950 sp 0x7ffddb44c948 READ of size 8 at 0x604000096810 thread T0 #0 0x7f491e356ac4 in nsCOMPtr<nsIAtom>::assign_with_AddRef(nsISupports*) /home/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:1105:5 #1 0x7f491e344c9d in nsCOMPtr<nsIAtom>::operator=(nsIAtom*) /home/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:593:5 #2 0x7f4923b1c764 in nsComputedDOMStyle::nsComputedDOMStyle(mozilla::dom::Element*, nsAString const&, nsIPresShell*, nsComputedDOMStyle::StyleType, nsComputedDOMStyle::AnimationFlag) /home/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:266:11 #3 0x7f4923b1c2ff in NS_NewComputedDOMStyle(mozilla::dom::Element*, nsAString const&, nsIPresShell*, nsComputedDOMStyle::StyleType, nsComputedDOMStyle::AnimationFlag) /home/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:75:23 #4 0x7f4920598418 in nsGlobalWindow::GetComputedStyleHelperOuter(mozilla::dom::Element&, nsAString const&, bool) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11337:5 #5 0x7f4920597fe2 in nsGlobalWindow::GetComputedStyleHelper(mozilla::dom::Element&, nsAString const&, bool, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11350:3 #6 0x7f49205980e3 in nsGlobalWindow::GetDefaultComputedStyle(mozilla::dom::Element&, nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11275:10 #7 0x7f492183ac24 in mozilla::dom::WindowBinding::getDefaultComputedStyle(JSContext*, JS::Handle<JSObject*>, nsGlobalWindow*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:4523:55 #8 0x7f49218315a4 in mozilla::dom::WindowBinding::genericMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:15690:13 #9 0x7f4926eb4a11 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /home/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15 #10 0x7f4926eb45bd in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:469:16 #11 0x7f4926eb5455 in InternalCall(JSContext*, js::AnyInvokeArgs const&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:514:12 #12 0x7f49270a9308 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/BaselineIC.cpp:2589:14 #13 0x39f990ced391 (<unknown module>) 0x604000096810 is located 0 bytes inside of 40-byte region [0x604000096810,0x604000096838) freed by thread T0 here: #0 0x4bc6ab in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3 #1 0x7f491e342392 in operator delete(void*) /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:224:12 #2 0x7f491e342392 in DynamicAtom::GCAtomTableLocked(mozilla::BaseAutoLock<mozilla::Mutex> const&, DynamicAtom::GCKind) /home/worker/workspace/build/src/xpcom/ds/nsAtomTable.cpp:437 #3 0x7f491e3420c0 in DynamicAtom::GCAtomTable() /home/worker/workspace/build/src/xpcom/ds/nsAtomTable.cpp:412:5 #4 0x7f491e34293f in DynamicAtom::Release() /home/worker/workspace/build/src/xpcom/ds/nsAtomTable.cpp:512:7 #5 0x7f4923aebde6 in nsCSSPseudoElements::GetPseudoAtom(nsAString const&) /home/worker/workspace/build/src/layout/style/nsCSSPseudoElements.cpp:152:1 #6 0x7f4923b1c758 in nsComputedDOMStyle::nsComputedDOMStyle(mozilla::dom::Element*, nsAString const&, nsIPresShell*, nsComputedDOMStyle::StyleType, nsComputedDOMStyle::AnimationFlag) /home/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:266:13 #7 0x7f4923b1c2ff in NS_NewComputedDOMStyle(mozilla::dom::Element*, nsAString const&, nsIPresShell*, nsComputedDOMStyle::StyleType, nsComputedDOMStyle::AnimationFlag) /home/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:75:23 #8 0x7f4920598418 in nsGlobalWindow::GetComputedStyleHelperOuter(mozilla::dom::Element&, nsAString const&, bool) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11337:5 #9 0x7f4920597fe2 in nsGlobalWindow::GetComputedStyleHelper(mozilla::dom::Element&, nsAString const&, bool, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11350:3 #10 0x7f49205980e3 in nsGlobalWindow::GetDefaultComputedStyle(mozilla::dom::Element&, nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11275:10 #11 0x7f492183ac24 in mozilla::dom::WindowBinding::getDefaultComputedStyle(JSContext*, JS::Handle<JSObject*>, nsGlobalWindow*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:4523:55 #12 0x7f49218315a4 in mozilla::dom::WindowBinding::genericMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:15690:13 #13 0x7f4926eb4a11 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /home/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15 #14 0x7f4926eb45bd in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:469:16 #15 0x7f4926eb5455 in InternalCall(JSContext*, js::AnyInvokeArgs const&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:514:12 #16 0x7f49270a9308 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/BaselineIC.cpp:2589:14 #17 0x39f990ced391 (<unknown module>) #18 0x62100125eedf (<unknown module>) #19 0x39f990ce0e3e (<unknown module>) #20 0x7f49270c0c14 in EnterBaseline(JSContext*, js::jit::EnterJitData&) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:160:9 #21 0x7f49270c17b0 in js::jit::EnterBaselineAtBranch(JSContext*, js::InterpreterFrame*, unsigned char*) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:268:28 #22 0x7f4926eae1bd in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2021:28 #23 0x7f4926e95b11 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:409:12 #24 0x7f4926eb7092 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:698:15 #25 0x7f4926eb7c22 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:730:12 #26 0x7f492776d56f in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) /home/worker/workspace/build/src/js/src/jsapi.cpp:4638:12 #27 0x7f492776ddd6 in ExecuteScript(JSContext*, JS::AutoObjectVector&, JS::Handle<JSScript*>, JS::Value*) /home/worker/workspace/build/src/js/src/jsapi.cpp:4657:12 #28 0x7f492776d9be in JS_ExecuteScript(JSContext*, JS::AutoObjectVector&, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:4678:12 #29 0x7f49208bfdeb in nsJSUtils::ExecutionContext::CompileAndExec(JS::CompileOptions&, JS::SourceBufferHolder&, JS::MutableHandle<JSScript*>) /home/worker/workspace/build/src/dom/base/nsJSUtils.cpp:265:8 #30 0x7f492364e1bf in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) /home/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2112:25 previously allocated by thread T0 here: #0 0x4bc9fc in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3 #1 0x4efb3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17 #2 0x7f491e34433d in operator new(unsigned long) /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:200:12 #3 0x7f491e34433d in DynamicAtom::Create(nsAString const&, unsigned int) /home/worker/workspace/build/src/xpcom/ds/nsAtomTable.cpp:78 #4 0x7f491e34152b in NS_Atomize(nsAString const&) /home/worker/workspace/build/src/xpcom/ds/nsAtomTable.cpp:772:30 #5 0x7f4923aebd6b in nsCSSPseudoElements::GetPseudoAtom(nsAString const&) /home/worker/workspace/build/src/layout/style/nsCSSPseudoElements.cpp:140:30 #6 0x7f4923b1c758 in nsComputedDOMStyle::nsComputedDOMStyle(mozilla::dom::Element*, nsAString const&, nsIPresShell*, nsComputedDOMStyle::StyleType, nsComputedDOMStyle::AnimationFlag) /home/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:266:13 #7 0x7f4923b1c2ff in NS_NewComputedDOMStyle(mozilla::dom::Element*, nsAString const&, nsIPresShell*, nsComputedDOMStyle::StyleType, nsComputedDOMStyle::AnimationFlag) /home/worker/workspace/build/src/layout/style/nsComputedDOMStyle.cpp:75:23 #8 0x7f4920598418 in nsGlobalWindow::GetComputedStyleHelperOuter(mozilla::dom::Element&, nsAString const&, bool) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11337:5 #9 0x7f4920597fe2 in nsGlobalWindow::GetComputedStyleHelper(mozilla::dom::Element&, nsAString const&, bool, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11350:3 #10 0x7f49205980e3 in nsGlobalWindow::GetDefaultComputedStyle(mozilla::dom::Element&, nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:11275:10 #11 0x7f492183ac24 in mozilla::dom::WindowBinding::getDefaultComputedStyle(JSContext*, JS::Handle<JSObject*>, nsGlobalWindow*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:4523:55 #12 0x7f49218315a4 in mozilla::dom::WindowBinding::genericMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WindowBinding.cpp:15690:13 #13 0x7f4926eb4a11 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /home/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15 #14 0x7f4926eb45bd in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:469:16 #15 0x7f4926eb5455 in InternalCall(JSContext*, js::AnyInvokeArgs const&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:514:12 #16 0x7f49270a9308 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/BaselineIC.cpp:2589:14 #17 0x39f990ced391 (<unknown module>) #18 0x62100125eedf (<unknown module>) #19 0x39f990ce0e3e (<unknown module>) #20 0x7f49270c0c14 in EnterBaseline(JSContext*, js::jit::EnterJitData&) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:160:9 #21 0x7f49270c17b0 in js::jit::EnterBaselineAtBranch(JSContext*, js::InterpreterFrame*, unsigned char*) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:268:28 #22 0x7f4926eae1bd in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2021:28 #23 0x7f4926e95b11 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:409:12 #24 0x7f4926eb7092 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:698:15 #25 0x7f4926eb7c22 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:730:12 #26 0x7f492776d56f in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) /home/worker/workspace/build/src/js/src/jsapi.cpp:4638:12 #27 0x7f492776ddd6 in ExecuteScript(JSContext*, JS::AutoObjectVector&, JS::Handle<JSScript*>, JS::Value*) /home/worker/workspace/build/src/js/src/jsapi.cpp:4657:12 #28 0x7f492776d9be in JS_ExecuteScript(JSContext*, JS::AutoObjectVector&, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:4678:12 #29 0x7f49208bfdeb in nsJSUtils::ExecutionContext::CompileAndExec(JS::CompileOptions&, JS::SourceBufferHolder&, JS::MutableHandle<JSScript*>) /home/worker/workspace/build/src/dom/base/nsJSUtils.cpp:265:8 #30 0x7f492364e1bf in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) /home/worker/workspace/build/src/dom/script/ScriptLoader.cpp:2112:25 SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:1105:5 in nsCOMPtr<nsIAtom>::assign_with_AddRef(nsISupports*) Shadow bytes around the buggy address: 0x0c088000acb0: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 0x0c088000acc0: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 00 0x0c088000acd0: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 0x0c088000ace0: fa fa 00 00 00 00 00 00 fa fa fd fd fd fd fd fa 0x0c088000acf0: fa fa fa fa fa fa fa fa fa fa 00 00 00 00 00 fa =>0x0c088000ad00: fa fa[fd]fd fd fd fd fa fa fa 00 00 00 00 00 fa 0x0c088000ad10: fa fa fa fa fa fa fa fa fa fa 00 00 00 00 00 00 0x0c088000ad20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c088000ad30: fa fa 00 00 00 00 00 fa fa fa fa fa fa fa fa fa 0x0c088000ad40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c088000ad50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==25979==ABORTING

Any ideas, heycam? I know you've been looking at style atom stuff.

It seems like nsCSSPseudoElements::GetPseudoAtom should really be returning an already_AddRefed<nsIAtom>. Currently we'll release the atom, which could make it fall to refcount 0. Much of the time we won't actually destroy the atom at this point, but if the release triggers an atom table GC, then it will.

I think this is a regression from bug 1210796 (and maybe something I should have caught while reviewing!). Before that bug, we were calling NS_Atomize and storing it directly in nsComputedDOMStyle::mPseudo, so we never had a zero refcount atom around.

(In reply to Cameron McCormack (:heycam) from comment #2) > It seems like nsCSSPseudoElements::GetPseudoAtom should really be returning > an already_AddRefed<nsIAtom>. Are there any pseudos that aren't static atoms?

You can call getComputedStyle with any pseudo name for the second argument, so we should be returning dynamic atoms for unrecognized pseudos, at least.

(Whether we need to store an unrecognized pseudo in nsComputedDOMStyle::mPseudo is a different question. We could have some other way to flag that we've got an unknown pseudo there. But currently we weed out unknown pseudos in nsComputedDOMStyle::DoGetStyleContextNoFlush.)

Jason, would you mind testing this patch for me?

[Tracking Requested - why for this release]:

Track 56+/57+ as sec-high.

(In reply to Cameron McCormack (:heycam) from comment #7) > Created attachment 8896104 [details] [diff] [review] > patch > > Jason, would you mind testing this patch for me? Apologies for the delay. I was unable to reproduce the issue using the patch.

Comment on attachment 8896104 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Not easily. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? 55 If not all supported branches, which bug introduced the flaw? bug 1210796 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? No, but it's likely this patch will apply easily. How likely is this patch to cause regressions; how much testing does it need? Very low. I think Jason's testing is sufficient here.

(In reply to Cameron McCormack (:heycam) from comment #11) > Which older supported branches are affected by this flaw? > > 55 That should say "55+".

sec-approval+ for trunk. We'll want a beta patch nominated as well.

Can you request uplift to beta ? Thanks.

Comment on attachment 8896104 [details] [diff] [review] patch Approval Request Comment [Feature/Bug causing the regression]: bug 1210796 [User impact if declined]: potential use-after-free of a DynamicAtom [Is this code covered by automated tests?]: the code around pseudo-elements is, yes [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: very small patch, just adds a strong reference to a local variable [String changes made/needed]: none

Comment on attachment 8896104 [details] [diff] [review] patch Fix a sec-high. Beta56+.

(In reply to Cameron McCormack (:heycam) from comment #17) > [Is this code covered by automated tests?]: the code around pseudo-elements > is, yes > [Has the fix been verified in Nightly?]: yes > [Needs manual test from QE? If yes, steps to reproduce]: no Setting qe-verify- based on Cameron's assessment on manual testing needs and the fact that this fix has automated coverage.