1346590 - heap-use-after-free [@ GetBoolFlag]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

The attached test case takes about 10 seconds to reproduce the crash.

==20467==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d00021db60 at pc 0x7ffa3b7fd2fc bp 0x7fffc19fe700 sp 0x7fffc19fe6f8
READ of size 4 at 0x60d00021db60 thread T0
    #0 0x7ffa3b7fd2fb in GetBoolFlag /home/worker/workspace/build/src/dom/base/nsINode.h:1596:12
    #1 0x7ffa3b7fd2fb in HasTextNodeDirectionalityMap /home/worker/workspace/build/src/dom/base/nsINode.h:1683
    #2 0x7ffa3b7fd2fb in RemoveElementFromMap /home/worker/workspace/build/src/dom/base/DirectionalityUtils.cpp:556
    #3 0x7ffa3b7fd2fb in mozilla::ResetDir(mozilla::dom::Element*) /home/worker/workspace/build/src/dom/base/DirectionalityUtils.cpp:1015
    #4 0x7ffa3b81307f in mozilla::dom::Element::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/base/Element.cpp:1880:5
    #5 0x7ffa3dbb018e in nsGenericHTMLElement::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/html/nsGenericHTMLElement.cpp:515:3
    #6 0x7ffa3b813183 in mozilla::dom::Element::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/base/Element.cpp:1893:7
    #7 0x7ffa3dbb018e in nsGenericHTMLElement::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/html/nsGenericHTMLElement.cpp:515:3
    #8 0x7ffa3b813183 in mozilla::dom::Element::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/base/Element.cpp:1893:7
    #9 0x7ffa3dbb018e in nsGenericHTMLElement::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/html/nsGenericHTMLElement.cpp:515:3
    #10 0x7ffa3db31839 in mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) /home/worker/workspace/build/src/dom/html/HTMLSharedElement.cpp:311:3
    #11 0x7ffa3b9df2ae in nsDocument::cycleCollection::Unlink(void*) /home/worker/workspace/build/src/dom/base/nsDocument.cpp:1892:5
    #12 0x7ffa3dbd591d in nsHTMLDocument::cycleCollection::Unlink(void*) /home/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:193:1
    #13 0x7ffa38df468f in nsCycleCollector::CollectWhite() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3331:5
    #14 0x7ffa38df7ae0 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3684:24
    #15 0x7ffa38dfadb2 in nsCycleCollector_collectSlice(js::SliceBudget&, bool) /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4173:3
    #16 0x7ffa3bb2016d in nsJSContext::RunCycleCollectorSlice() /home/worker/workspace/build/src/dom/base/nsJSEnvironment.cpp:1488:3
    #17 0x7ffa38f615c2 in nsTimerImpl::Fire(int) /home/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp:484:7
    #18 0x7ffa38f2f68b in nsTimerEvent::Run() /home/worker/workspace/build/src/xpcom/threads/TimerThread.cpp:288:3
    #19 0x7ffa38f531b2 in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1264:7
    #20 0x7ffa38f4fa60 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #21 0x7ffa39d61cbf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #22 0x7ffa39cd38c8 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:3
    #23 0x7ffa39cd38c8 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #24 0x7ffa39cd38c8 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #25 0x7ffa3f1bfbcf in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:3
    #26 0x7ffa428284b1 in nsAppStartup::Run() /home/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:283:19
    #27 0x7ffa429f86cc in XREMain::XRE_mainRun() /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4476:10
    #28 0x7ffa429fa1cc in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4654:8
    #29 0x7ffa429fb5cc in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4745:16
    #30 0x4dffaf in do_main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:236:10
    #31 0x4dffaf in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:307
    #32 0x7ffa5440d82f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291
    #33 0x41c3d8 in _start (/home/user/workspace/browsers/firefox_cnt/firefox+0x41c3d8)

0x60d00021db60 is located 48 bytes inside of 144-byte region [0x60d00021db30,0x60d00021dbc0)
freed by thread T0 here:
    #0 0x4b2b2b in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3
    #1 0x7ffa38f6a0be in operator delete /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:218:12
    #2 0x7ffa38f6a0be in Release /home/worker/workspace/build/src/xpcom/reflect/xptinfo/ShimInterfaceInfo.cpp:462
    #3 0x7ffa38f6a0be in Release /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:40
    #4 0x7ffa38f6a0be in Release /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:395
    #5 0x7ffa38f6a0be in ~RefPtr /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:78
    #6 0x7ffa38f6a0be in xptiInterfaceEntry::GetIIDForParamNoAlloc(unsigned short, nsXPTParamInfo const*, nsID*) /home/worker/workspace/build/src/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp:443
    #7 0x7ffa3a7dedf1 in GetInterfaceTypeFromParam /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1441:14
    #8 0x7ffa3a7dedf1 in GatherAndConvertResults /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1525
    #9 0x7ffa3a7dedf1 in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1340
    #10 0x7ffa3a7dedf1 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1296
    #11 0x7ffa3a7e6427 in GetAttribute /home/worker/workspace/build/src/js/xpconnect/src/xpcprivate.h:1679:17
    #12 0x7ffa3a7e6427 in XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1019
    #13 0x18f0597abd98  (<unknown module>)
    #14 0x7ffa4328af76 in EnterIon /home/worker/workspace/build/src/js/src/jit/Ion.cpp:2898:9
    #15 0x7ffa4328af76 in js::jit::IonCannon(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/jit/Ion.cpp:2996
    #16 0x7ffa42e3f091 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:374:41
    #17 0x7ffa42e73496 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #18 0x7ffa42e74849 in InternalCall /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:493:12
    #19 0x7ffa42e74849 in Call /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512
    #20 0x7ffa42e74849 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:627
    #21 0x7ffa43d8370e in CallGetter /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1832:16
    #22 0x7ffa43d8370e in GetExistingProperty<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1880
    #23 0x7ffa43d8370e in NativeGetPropertyInline<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2111
    #24 0x7ffa43d8370e in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2145
    #25 0x7ffa42e7bdbe in GetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1442:12
    #26 0x7ffa42e7bdbe in GetProperty /home/worker/workspace/build/src/js/src/jsobj.h:845
    #27 0x7ffa42e7bdbe in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:4307
    #28 0x7ffa43610e59 in js::jit::ComputeGetPropResult(JSContext*, js::jit::BaselineFrame*, JSOp, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/SharedIC.cpp:1997:18
    #29 0x7ffa435fec86 in js::jit::DoGetPropGeneric(JSContext*, js::jit::BaselineFrame*, js::jit::ICGetProp_Generic*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/SharedIC.cpp:2169:12
    #30 0x18f0596a0a3b  (<unknown module>)
    #31 0x621001a10d0f  (<unknown module>)
    #32 0x18f05969c8a5  (<unknown module>)
    #33 0x7ffa430c0941 in EnterBaseline(JSContext*, js::jit::EnterJitData&) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:160:9
    #34 0x7ffa430c015c in js::jit::EnterBaselineMethod(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:200:28
    #35 0x7ffa42e3f0f0 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:384:41
    #36 0x7ffa42e73496 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #37 0x7ffa42e73b72 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
    #38 0x7ffa438ebd9b in js::fun_apply(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/jsfun.cpp:1269:12
    #39 0x7ffa42e7317f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #40 0x7ffa42e7317f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #41 0x7ffa42e73b72 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
    #42 0x7ffa43635f2c in js::jit::InvokeFunction(JSContext*, JS::Handle<JSObject*>, bool, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/VMFunctions.cpp:114:12
    #43 0x18f0596a5e9f  (<unknown module>)

previously allocated by thread T0 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 0x7ffa38f66bf0 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:194:12
    #3 0x7ffa38f66bf0 in ShimInterfaceInfo::MaybeConstruct(char const*, JSContext*) /home/worker/workspace/build/src/xpcom/reflect/xptinfo/ShimInterfaceInfo.cpp:472
    #4 0x7ffa38f696d4 in xptiInterfaceEntry::GetShimForParam(unsigned short, nsXPTParamInfo const*) /home/worker/workspace/build/src/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp:382:9
    #5 0x7ffa38f69ff0 in xptiInterfaceEntry::GetIIDForParamNoAlloc(unsigned short, nsXPTParamInfo const*, nsID*) /home/worker/workspace/build/src/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp:433:42
    #6 0x7ffa3a7dedf1 in GetInterfaceTypeFromParam /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1441:14
    #7 0x7ffa3a7dedf1 in GatherAndConvertResults /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1525
    #8 0x7ffa3a7dedf1 in Call /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1340
    #9 0x7ffa3a7dedf1 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1296
    #10 0x7ffa3a7e6427 in GetAttribute /home/worker/workspace/build/src/js/xpconnect/src/xpcprivate.h:1679:17
    #11 0x7ffa3a7e6427 in XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1019
    #12 0x18f0597abd98  (<unknown module>)
    #13 0x7ffa4328af76 in EnterIon /home/worker/workspace/build/src/js/src/jit/Ion.cpp:2898:9
    #14 0x7ffa4328af76 in js::jit::IonCannon(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/jit/Ion.cpp:2996
    #15 0x7ffa42e3f091 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:374:41
    #16 0x7ffa42e73496 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #17 0x7ffa42e74849 in InternalCall /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:493:12
    #18 0x7ffa42e74849 in Call /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512
    #19 0x7ffa42e74849 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:627
    #20 0x7ffa43d8370e in CallGetter /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1832:16
    #21 0x7ffa43d8370e in GetExistingProperty<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1880
    #22 0x7ffa43d8370e in NativeGetPropertyInline<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2111
    #23 0x7ffa43d8370e in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2145
    #24 0x7ffa42e7bdbe in GetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1442:12
    #25 0x7ffa42e7bdbe in GetProperty /home/worker/workspace/build/src/js/src/jsobj.h:845
    #26 0x7ffa42e7bdbe in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:4307
    #27 0x7ffa43610e59 in js::jit::ComputeGetPropResult(JSContext*, js::jit::BaselineFrame*, JSOp, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/SharedIC.cpp:1997:18
    #28 0x7ffa435fec86 in js::jit::DoGetPropGeneric(JSContext*, js::jit::BaselineFrame*, js::jit::ICGetProp_Generic*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/SharedIC.cpp:2169:12
    #29 0x18f0596a0a3b  (<unknown module>)
    #30 0x621001a10d0f  (<unknown module>)
    #31 0x18f05969c8a5  (<unknown module>)
    #32 0x7ffa430c0941 in EnterBaseline(JSContext*, js::jit::EnterJitData&) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:160:9
    #33 0x7ffa430c015c in js::jit::EnterBaselineMethod(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/jit/BaselineJIT.cpp:200:28
    #34 0x7ffa42e3f0f0 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:384:41
    #35 0x7ffa42e73496 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #36 0x7ffa42e73b72 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
    #37 0x7ffa438ebd9b in js::fun_apply(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/js/src/jsfun.cpp:1269:12
    #38 0x7ffa42e7317f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #39 0x7ffa42e7317f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #40 0x7ffa42e73b72 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
    #41 0x7ffa43635f2c in js::jit::InvokeFunction(JSContext*, JS::Handle<JSObject*>, bool, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/VMFunctions.cpp:114:12
    #42 0x18f0596a5e9f  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/dom/base/nsINode.h:1596:12 in GetBoolFlag
Shadow bytes around the buggy address:
  0x0c1a8003bb10: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1a8003bb20: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x0c1a8003bb30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1a8003bb40: fa fa fa fa fa fa fa fa fa fa fa fa 00 00 00 00
  0x0c1a8003bb50: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
=>0x0c1a8003bb60: fa fa fa fa fa fa fd fd fd fd fd fd[fd]fd fd fd
  0x0c1a8003bb70: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c1a8003bb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1a8003bb90: 00 fa fa fa fa fa fa fa fa fa 00 00 00 00 00 00
  0x0c1a8003bba0: 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c1a8003bbb0: fa fa fa fa 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
==20467==ABORTING

Comment 1

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: c1a5c44ae3d8 (2013-03-13)
INFO: First bad revision: b672877ed046 (2013-03-14)
INFO: Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c1a5c44ae3d8&tochange=b672877ed046

Note: To reproduce the crash on <44, you need to s/let/var in the testcase.

Comment 2

[Andrew McCreight [:mccr8]]

There's nothing obviously related in the regression range in comment 1.

The use stack looks like bug 827190. The most obviously unusual part of the test case is the ruby and bdi stuff.

It looks like the object being touched after it was destroyed is an nsTextNode. The allocation and freeing stacks look similar, so somehow we're making an XPCWN call, we create and destroy a text node, but end up with a dangling pointer in an element?

Comment 3

[Mats Palmgren (inactive)]

Fwiw, I seem to recall there were some unresolved issues with that code.
Bug 836925 / bug 900997 might have some clues.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Smells similar to bug 1289970

Comment 5

[Gerry Chang [:gchang]]

Track 53+/54+/55+ as security related issue.

Comment 6

[Andrew Overholt [:overholt]]

Ehsan, do you have any thoughts here that might help someone investigate?

Comment 7

[(no longer active)]

There is a huge comment in the beginning of DirectionalityUtils.cpp that explains how this all works.  I'd much rather someone else own this bug though, since I can't be sure how long this will take to debug and when I can get to it.

But on a higher level, a lot of the security bugs in this code over the years have come from this member holding on to raw pointers: <http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/dom/base/DirectionalityUtils.cpp#477>  Olli, do you think it's time for us to finally bite the bullet and switch to using strong refs there?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I think I've been looking at this enough that I should just take this.

I'm not sure about the strong pointers yet. The setup is complicated so I'm a bit worried about leaks too.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: a variant of the testcase

This doesn't have dir attribute in bdi initial

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

This fixes these particular issues, but as far as I see, there is still some other more underlying bug.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

FWIW, I expect fixing this will take still several days.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

removing dir=auto from bdi shouldn't affect to dirAutoSetBy, but it does

Comment 13

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late for a fix for 53, but we could still take a patch for 54.

Comment 15

[Stephanie Ouillon [:arroway] (needinfo me)]

Olli, what's the status on this bug?

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

I need to find several days (I expect) free time for this and bug 1360893.

Perhaps I should look at this again later today, and see if some quick hack could be added.

Comment 17

[Frederik Braun [:freddy]]

I understand you must be busy.
Is there anything we can do to unblock you or take this further ourselves while we wait?

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Attachment: dirty but safe approach

Keep strong reference to nsTextNode in properties, cycle collected it and release when deleting the property.
And because deletion may happen during enumeration, add mElementToBeRemoved.

Also ensure objects are correctly kept alive, so use one nsCOMPtr and RefPtr.

I'd say this is dirty approach, but this code has had so many issues over the time that we need less error prone setup, and this should be relatively branch-safe too.

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

The patch fixes bug 1360893 too.

Comment 21

[(no longer active)]

Comment on attachment 8874395 [details] [diff] [review]
dirty but safe approach

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

This is better than fixing one of these bugs every few months!  Thanks for the patch.

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8874395 [details] [diff] [review]
dirty but safe approach

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
I'd say not very easily, but the patch does fix two bugs filed by different people.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Commit message could be
"Bug 1346590 simplify directionality handling by relying on cycle collector, r=ehsan"

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?
Seems to apply to older branches too. 

How likely is this patch to cause regressions; how much testing does it need?
The risk here is leaking, which I tried to test locally. An earlier version of the patch was leaking because not
all the UnsetProperty calls were changed to use DeleteProperty, but would be a bit surprising to leak now that they have
been converted.

Comment 23

[Al Billings [:abillings - ex-MoCo]]

I'll give sec-approval to check in on June 27, two weeks into the new cycle. We're making the release candidate for Firefox 54 today and have run out of betas for this release. We'll want patches nominated for Beta and ESR52 when it does go in during late June.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b99ff3a0012c57ffab1fa291234be3f4428d5875

This grafts cleanly to Beta and ESR52. Please request approval when you get a chance.

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8874395 [details] [diff] [review]
dirty but safe approach

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: See comment 22
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): see comment 22
String or UUID changes made by this patch: NA

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b99ff3a0012c

seems also need beta uplift request

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8874395 [details] [diff] [review]
dirty but safe approach

Approval Request Comment
See comment 22

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/abd7d1893a23

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/f19b6c6a0c6c61a19f649d1c6ecb68dcb00aa321