1400628 - crash near null in [@ mozilla::a11y::HTMLSelectListAccessible::IsAcceptableChild]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

==5763==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f0e3889e14c bp 0x7ffd166d1b90 sp 0x7ffd166d1b90 T0)
==5763==The signal is caused by a READ memory access.
==5763==Hint: address points to the zero page.
    #0 0x7f0e3889e14b in get /src/obj-firefox/dist/include/mozilla/RefPtr.h:287:27
    #1 0x7f0e3889e14b in operator-> /src/obj-firefox/dist/include/mozilla/RefPtr.h:319
    #2 0x7f0e3889e14b in IsInNamespace /src/obj-firefox/dist/include/nsINode.h:608
    #3 0x7f0e3889e14b in IsHTMLElement /src/obj-firefox/dist/include/nsIContent.h:279
    #4 0x7f0e3889e14b in IsAnyOfHTMLElements<nsIAtom *, nsIAtom *> /src/obj-firefox/dist/include/nsIContent.h:290
    #5 0x7f0e3889e14b in mozilla::a11y::HTMLSelectListAccessible::IsAcceptableChild(nsIContent*) const /src/accessible/html/HTMLSelectAccessible.cpp:121
    #6 0x7f0e38868016 in mozilla::a11y::DocAccessible::AddDependentIDsFor(mozilla::a11y::Accessible*, nsIAtom*) /src/accessible/generic/DocAccessible.cpp:1612:26
    #7 0x7f0e316ae6d7 in nsNodeUtils::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int, nsAttrValue const*) /src/dom/base/nsNodeUtils.cpp:146:3
    #8 0x7f0e31354fa1 in mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const*, nsAttrValue&, unsigned char, bool, bool, bool, nsIDocument*, mozAutoDocUpdate const&) /src/dom/base/Element.cpp:2673:5
    #9 0x7f0e31356d9c in mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsTSubstring<char16_t> const&, bool) /src/dom/base/Element.cpp:2499:10
    #10 0x7f0e3134bf40 in SetAttr /src/obj-firefox/dist/include/mozilla/dom/Element.h:671:12
    #11 0x7f0e3134bf40 in mozilla::dom::Element::SetAttribute(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /src/dom/base/Element.cpp:1288
    #12 0x7f0e32b8cf1c in mozilla::dom::ElementBinding::setAttribute(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/ElementBinding.cpp:748:9
    #13 0x7f0e3308c5e0 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3055:13
    #14 0x7f0e396ee864 in CallJSNative /src/js/src/jscntxtinlines.h:293:15
    #15 0x7f0e396ee864 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495
    #16 0x7f0e396d83ff in CallFromStack /src/js/src/vm/Interpreter.cpp:546:12
    #17 0x7f0e396d83ff in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3084
    #18 0x7f0e396bf98b in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:435:12
    #19 0x7f0e396ee9fc in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:513:15
    #20 0x7f0e396ef352 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:559:10
    #21 0x7f0e3a14571b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2965:12
    #22 0x7f0e32ac13c5 in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
    #23 0x7f0e33498005 in Call<nsISupports *> /src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
    #24 0x7f0e33498005 in mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /src/dom/events/JSEventHandler.cpp:215
    #25 0x7f0e334615d9 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1112:51
    #26 0x7f0e334636a0 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1283:20
    #27 0x7f0e33443491 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16
    #28 0x7f0e33446962 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:822:9
    #29 0x7f0e357113ee in nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1081:7
    #30 0x7f0e386bbec1 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:7741:21
    #31 0x7f0e386b7ee4 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7539:7
    #32 0x7f0e386bf88f in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7436:13
    #33 0x7f0e303c61d0 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1320:3
    #34 0x7f0e303c528c in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:861:14
    #35 0x7f0e303c2246 in nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:750:9
    #36 0x7f0e303c225f in ChildDoneWithOnload /src/uriloader/base/nsDocLoader.h:201:9
    #37 0x7f0e303c225f in nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:753
    #38 0x7f0e303c4085 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:632:5
    #39 0x7f0e303c4cec in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:488:14
    #40 0x7f0e2ebc849d in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:629:28
    #41 0x7f0e31595cdd in nsDocument::DoUnblockOnload() /src/dom/base/nsDocument.cpp:9171:18
    #42 0x7f0e315958a1 in nsDocument::UnblockOnload(bool) /src/dom/base/nsDocument.cpp:9093:9
    #43 0x7f0e3156efd9 in nsDocument::DispatchContentLoadedEvents() /src/dom/base/nsDocument.cpp:5597:3
    #44 0x7f0e3160fa02 in applyImpl<nsDocument, void (nsDocument::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1142:12
    #45 0x7f0e3160fa02 in apply<nsDocument, void (nsDocument::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1148
    #46 0x7f0e3160fa02 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1192
    #47 0x7f0e2ea1b4fd in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
    #48 0x7f0e2ea21238 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:521:10
    #49 0x7f0e2f7c4d31 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
    #50 0x7f0e2f726bfb in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10
    #51 0x7f0e2f726bfb in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319
    #52 0x7f0e2f726bfb in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299
    #53 0x7f0e34ec5cef in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
    #54 0x7f0e39020b21 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #55 0x7f0e3920158b in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4701:22
    #56 0x7f0e39203188 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4865:8
    #57 0x7f0e392045bb in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4960:21
    #58 0x4ebea3 in do_main /src/browser/app/nsBrowserApp.cpp:236:22
    #59 0x4ebea3 in main /src/browser/app/nsBrowserApp.cpp:309
    #60 0x7f0e4c3aa82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #61 0x41d9f8 in _start (/home/ubuntu/firefox/firefox+0x41d9f8)

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Check if dependent content is null before checking if it is accepteptable. r?surkov

Oof, that's a bad one. Good catch.

Comment 2

[Eitan Isaacson [:eeejay]]

This looks like bug 1330739. The patch there fixes this on ESR.

Comment 3

[Eitan Isaacson [:eeejay]]

Disregard last comment. This is not a security bug and the previous patch is not for this bug. I commented in the wrong place..

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41071f38501c

Can we land a test too?

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Add test for non existing ID in select[aria-owns]. r?surkov

Good idea.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 8909489 [details] [diff] [review]
Add test for non existing ID in select[aria-owns]. r?surkov

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

::: accessible/tests/browser/tree/browser_aria_owns.js
@@ +159,5 @@
>  );
>  
> +// Don't crash if ID in aria-owns does not exist
> +addAccessibleTask(`
> +  <select id="container" aria-owns="boom" multiple></select>`,

select is sort of special, it makes sense to use something more general (in case if later we change aria-owns processing on 'select', for example, if we started to ignore aria-onws on it at all)

Comment 7

[Eitan Isaacson [:eeejay]]

This crash only occurs on a select[multiple] element because of its IsAcceptableChild implementation.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d783881ff247
Add test for non existing ID in select[aria-owns]. r=surkov

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/41071f38501c

Comment 10

[alexander :surkov (:asurkov)]

(In reply to Eitan Isaacson [:eeejay] from comment #7)
> This crash only occurs on a select[multiple] element because of its
> IsAcceptableChild implementation.

oh, right Accessible::IsAcceptableChild implementation has a null aEl check for some reason. Do you think we need it considering the approach you had taken in the patch?

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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