Closed Bug 865076 Opened 12 years ago Closed 12 years ago

Heap-use-after-free in nsAttrAndChildArray::GetAttr

Categories

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

x86_64
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla23
Tracking Status
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: inferno, Assigned: smaug)

References

Details

(6 keywords, Whiteboard: [asan][adv-main23-])

Attachments

(2 files)

Attached file Testcase (deleted) —
Need the fuzzPriv extension to forceGC ==7961== ERROR: AddressSanitizer: heap-use-after-free on address 0x603800ddc660 at pc 0x7fa52c52e33e bp 0x7fffd07dc730 sp 0x7fffd07dc728 READ of size 8 at 0x603800ddc660 thread T0 #0 0x7fa52c52e33d in nsAttrAndChildArray::AttrSlotCount() const content/base/src/nsAttrAndChildArray.h:144 #1 0x7fa52c52d68b in nsAttrAndChildArray::GetAttr(nsIAtom*, int) const content/base/src/nsAttrAndChildArray.cpp:297 #2 0x7fa52b21200c in mozilla::dom::Element::AttrValueIs(int, nsIAtom*, nsIAtom*, nsCaseTreatment) const ../../../dist/include/mozilla/dom/Element.h:1189 #3 0x7fa53190bd2e in mozilla::dom::SVGSwitchElement::FindActiveChild() const content/svg/content/src/SVGSwitchElement.cpp:129 #4 0x7fa53190b82d in mozilla::dom::SVGSwitchElement::MaybeInvalidate() content/svg/content/src/SVGSwitchElement.cpp:59 #5 0x7fa53190e041 in mozilla::dom::SVGSwitchElement::RemoveChildAt(unsigned int, bool) content/svg/content/src/SVGSwitchElement.cpp:99 #6 0x7fa52cb68104 in nsINode::Remove() content/base/src/nsINode.cpp:1408 #7 0x7fa535e8de5a in mozilla::dom::ElementBinding::remove(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:2009 #8 0x7fa535e8d53a in mozilla::dom::ElementBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:2055 #9 0x7fa53fb7accf in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:337 #10 0x7fa53fb7accf in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:407 #11 0x7fa53fb541a0 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) js/src/jsinterp.cpp:2396 #12 0x7fa53faff64d in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:364 #13 0x7fa53fb7b378 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:421 #14 0x7fa53f59c836 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:134 #15 0x7fa53fb7edd0 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:454 #16 0x7fa53f4b3490 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5891 #17 0x7fa531d9cd87 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1445 #18 0x7fa531d4fc4c in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:578 #19 0x7fa5380ca624 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122 #20 0x7fa5380c76da in SharedStub 0x603800ddc660 is located 96 bytes inside of 232-byte region [0x603800ddc600,0x603800ddc6e8) freed by thread T0 here: #0 0x4183f2 in __interceptor_free #1 0x7fa5487816ce in moz_free memory/mozalloc/mozalloc.cpp:48 #2 0x7fa53190e4d9 in operator delete(void*) ../../../../dist/include/mozilla/mozalloc.h:225 #3 0x7fa53190e4d9 in mozilla::dom::SVGSwitchElement::~SVGSwitchElement() ../../../../dist/include/mozilla/dom/SVGSwitchElement.h:19 #4 0x7fa52cc319c5 in nsNodeUtils::LastRelease(nsINode*) content/base/src/nsNodeUtils.cpp:258 #5 0x7fa52cedc2fa in mozilla::dom::FragmentOrElement::Release() content/base/src/FragmentOrElement.cpp:1716 #6 0x7fa531511d45 in nsSVGElement::Release() content/svg/content/src/nsSVGElement.cpp:228 #7 0x7fa5317b7905 in mozilla::dom::SVGGraphicsElement::Release() content/svg/content/src/SVGGraphicsElement.cpp:15 #8 0x7fa53190a075 in mozilla::dom::SVGSwitchElement::Release() content/svg/content/src/SVGSwitchElement.cpp:37 #9 0x7fa52ca9a106 in mozilla::dom::Element::UnbindFromTree(bool, bool) content/base/src/Element.cpp:1283 #10 0x7fa52d90caa1 in nsGenericHTMLElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:655 #11 0x7fa52cb68969 in nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) content/base/src/nsINode.cpp:1437 #12 0x7fa52cec95e5 in mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) content/base/src/FragmentOrElement.cpp:924 #13 0x7fa53190e037 in mozilla::dom::SVGSwitchElement::RemoveChildAt(unsigned int, bool) content/svg/content/src/SVGSwitchElement.cpp:98 #14 0x7fa52cb68104 in nsINode::Remove() content/base/src/nsINode.cpp:1408 #15 0x7fa535e8de5a in mozilla::dom::ElementBinding::remove(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:2009 #16 0x7fa535e8d53a in mozilla::dom::ElementBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:2055 #17 0x7fa53fb7accf in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:337 #18 0x7fa53fb7accf in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:407 #19 0x7fa53fb541a0 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) js/src/jsinterp.cpp:2396 #20 0x7fa53faff64d in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:364 #21 0x7fa53fb7b378 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:421 #22 0x7fa53f59c836 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:134 #23 0x7fa53fb7edd0 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:454 #24 0x7fa53f4b3490 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5891 #25 0x7fa531d9cd87 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1445 #26 0x7fa531d4fc4c in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:578 #27 0x7fa5380ca624 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122 #28 0x7fa5380c76da in SharedStub #29 0x7fa52d54936c in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:944 #30 0x7fa52d54b91f in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:1015 #31 0x7fa52d740ceb in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.h:326 previously allocated by thread T0 here: #0 0x4184d2 in malloc #1 0x7fa548781815 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54 #2 0x7fa531908360 in operator new(unsigned long) ../../../../dist/include/mozilla/mozalloc.h:201 #3 0x7fa531908360 in NS_NewSVGSwitchElement(nsIContent**, already_AddRefed<nsINodeInfo>) content/svg/content/src/SVGSwitchElement.cpp:13 #4 0x7fa5316fb8e8 in CreateSwitchElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/svg/content/src/SVGTagList.h:85 #5 0x7fa5316edd78 in NS_NewSVGElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/svg/content/src/SVGElementFactory.cpp:126 #6 0x7fa52cc010ee in NS_NewElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/base/src/nsNameSpaceManager.cpp:203 #7 0x7fa52c880576 in nsIDocument::CreateElementNS(nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) content/base/src/nsDocument.cpp:4767 #8 0x7fa535d47756 in mozilla::dom::DocumentBinding::createElementNS(JSContext*, JS::Handle<JSObject*>, nsIDocument*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/DocumentBinding.cpp:524 #9 0x7fa535cae35a in mozilla::dom::DocumentBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/DocumentBinding.cpp:7645 #10 0x7fa53fb7accf in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:337 #11 0x7fa53fb7accf in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:407 #12 0x7fa53fb541a0 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) js/src/jsinterp.cpp:2396 #13 0x7fa53faff64d in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:364 #14 0x7fa53fb7b378 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:421 #15 0x7fa53f59c836 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:134 #16 0x7fa53fb7edd0 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:454 #17 0x7fa53f4b3490 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5891 #18 0x7fa531d9cd87 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1445 #19 0x7fa531d4fc4c in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:578 #20 0x7fa5380ca624 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122 #21 0x7fa5380c76da in SharedStub #22 0x7fa52d54936c in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:944 #23 0x7fa52d54b91f in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:1015 #24 0x7fa52d740ceb in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.h:326 #25 0x7fa52d730aa7 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, bool, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:200 #26 0x7fa52d72e46c in nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, nsDispatchingCallback*, bool, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:325 #27 0x7fa52d736131 in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) content/events/src/nsEventDispatcher.cpp:631 #28 0x7fa52d7386dd in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) content/events/src/nsEventDispatcher.cpp:691 #29 0x7fa52cb61ca4 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) content/base/src/nsINode.cpp:1136 #30 0x7fa52c6692db in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3620 #31 0x7fa52c668644 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3590 Shadow bytes around the buggy address: 0x0c07801b3870: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c07801b3880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c07801b3890: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c07801b38a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c07801b38b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c07801b38c0: fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd 0x0c07801b38d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa 0x0c07801b38e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c07801b38f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c07801b3900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c07801b3910: 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 righ 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 ASan internal: fe ==7961== ABORTING
The SVGSwitchElement is deleted. BTW, here's the add-on I used: https://www.squarefree.com/extensions/domFuzzLite3.xpi
Severity: normal → critical
Component: General → SVG
Product: Firefox → Core
Whiteboard: [asan]
Curious that nsINode::Remove() calls SVGSwitchElement::RemoveChildAt(), when looking at the script, the test1.attributes[0].ownerElement.remove(); should be calling remove() on the <url> element. Just before the remove() call, the document should look like: <thead><switch><url><script>...</script></url></switch></thead> Once the <switch> gets GCed, so should the <url>. I don't think the "test1" should resolve to the <url> element. Instead it should throw a ReferenceError. I wonder if this is an issue with the Global Scope Polluter?
Actually the <thead> isn't even in the document, so it shouldn't resolve anyway...
Actually, ignore that, and the curiosity in comment 2. The first time "test1" is referenced -- in the appendChild() call -- causes the GSP property to spring into existence. So I guess that holds a strong reference to the <url> element. And of course remove() is equivalent to if (this.parentNode) this.parentNode.removeChild(this); so it almost makes sense that it's calling RemoveChildAt() on the SVGSwitchElement. So: even though the <switch> is removed from the document, it shouldn't have been GCed, since it should still be accessible through <url>.parentNode.
nsINode::Remove is wrong. It must keep the parent alive
Assignee: nobody → bugs
Blocks: 856629
Attached patch patch (deleted) — Splinter Review
Attachment #741210 - Flags: review?(Ms2ger)
Component: SVG → DOM
Attachment #741210 - Flags: review?(Ms2ger) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Whiteboard: [asan] → [asan][adv-main23-]
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Attachment #808832 - Attachment description: Bug Bounty Awarded $3000 → Bug Bounty Awarded $3000 [paid] 9/25/2013
Group: core-security
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: