Closed Bug 1510485 Opened 6 years ago Closed 5 years ago

crash near null in [@ nsWebBrowserFind::SetSelectionAndScroll]

Categories

(Toolkit :: Find Toolbar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- unaffected
firefox65 + fixed
firefox66 --- fixed

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(2 files)

Attached file testcase.html
==41939==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000001c (pc 0x7f351fbe7a90 bp 0x7ffed8cab2b0 sp 0x7ffed8cab100 T0)
==41939==The signal is caused by a READ memory access.
==41939==Hint: address points to the zero page.
    #0 0x7f351fbe7a8f in GetBoolFlag src/obj-firefox/dist/include/nsINode.h:1593:12
    #1 0x7f351fbe7a8f in IsInUncomposedDoc src/obj-firefox/dist/include/nsINode.h:630
    #2 0x7f351fbe7a8f in GetPrimaryFrame src/obj-firefox/dist/include/nsIContent.h:684
    #3 0x7f351fbe7a8f in nsWebBrowserFind::SetSelectionAndScroll(nsPIDOMWindowOuter*, nsRange*) src/toolkit/components/find/nsWebBrowserFind.cpp:364
    #4 0x7f351fbe6acd in nsWebBrowserFind::SearchInFrame(nsPIDOMWindowOuter*, bool, bool*) src/toolkit/components/find/nsWebBrowserFind.cpp:729:5
    #5 0x7f351fbe4b16 in nsWebBrowserFind::FindNext(bool*) src/toolkit/components/find/nsWebBrowserFind.cpp:112:8
    #6 0x7f35155db552 in nsGlobalWindowOuter::FindOuter(nsTSubstring<char16_t> const&, bool, bool, bool, bool, bool, bool, mozilla::ErrorResult&) src/dom/base/nsGlobalWindowOuter.cpp:6482:20
    #7 0x7f3517b9bb4e in mozilla::dom::Window_Binding::find(JSContext*, JS::Handle<JSObject*>, nsGlobalWindowInner*, JSJitMethodCallArgs const&) src/obj-firefox/dom/bindings/WindowBinding.cpp:6118:21
    #8 0x7f3518af3081 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::MaybeGlobalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3376:13
    #9 0x7f352205637d in CallJSNative src/js/src/vm/Interpreter.cpp:468:15
    #10 0x7f352205637d in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:560
    #11 0x7f3522040007 in CallFromStack src/js/src/vm/Interpreter.cpp:620:12
    #12 0x7f3522040007 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3494
    #13 0x7f35220235c6 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:447:12
    #14 0x7f3522056d21 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:587:15
    #15 0x7f35220589a2 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:634:10
    #16 0x7f352101bf46 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2994:12
    #17 0x7f35180fdd29 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:265:37
    #18 0x7f3519392329 in void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:363:12
    #19 0x7f351938f5b9 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) src/dom/events/JSEventHandler.cpp:214:12
    #20 0x7f35193436fa in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1115:52
    #21 0x7f3519345cf7 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1317:15
    #22 0x7f35193274a6 in HandleEvent src/obj-firefox/dist/include/mozilla/EventListenerManager.h:390:5
    #23 0x7f35193274a6 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:425
    #24 0x7f3519325728 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:642:16
    #25 0x7f351932c180 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1164:11
    #26 0x7f351c09cd8e in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1164:7
    #27 0x7f351f43eae3 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:7044:21
    #28 0x7f351f43a309 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:6835:7
    #29 0x7f351f443107 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
    #30 0x7f3513fde9c5 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1310:3
    #31 0x7f3513fdd5ac in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:853:14
    #32 0x7f3513fd8ef8 in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:742:9
    #33 0x7f3513fdb84e in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:631:5
    #34 0x7f3513fdd0d4 in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp
    #35 0x7f35118ffe17 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:628:28
    #36 0x7f35152a20cd in imgRequestProxy::RemoveFromLoadGroup() src/image/imgRequestProxy.cpp:440:15
    #37 0x7f35152abc6a in imgRequestProxy::OnLoadComplete(bool) src/image/imgRequestProxy.cpp:1084:7
    #38 0x7f351528a0ec in operator() src/image/ProgressTracker.cpp:358:13
    #39 0x7f351528a0ec in void mozilla::image::ImageObserverNotifier<mozilla::image::ObserverTable const*>::operator()<void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::'lambda5'(mozilla::image::IProgressObserver*)>(mozilla::image::ObserverTable const*) src/image/ProgressTracker.cpp:283
    #40 0x7f3515287896 in void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) src/image/ProgressTracker.cpp:357:5
    #41 0x7f35151e4540 in operator() src/image/ProgressTracker.cpp:378:5
    #42 0x7f35151e4540 in Read<(lambda at src/image/ProgressTracker.cpp:377:19)> src/image/CopyOnWrite.h:179
    #43 0x7f35151e4540 in mozilla::image::ProgressTracker::SyncNotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) src/image/ProgressTracker.cpp:377
    #44 0x7f35152383b8 in mozilla::image::VectorImage::OnSVGDocumentLoaded() src/image/VectorImage.cpp:1551:23
    #45 0x7f35152697d1 in mozilla::image::SVGLoadEventListener::HandleEvent(mozilla::dom::Event*) src/image/VectorImage.cpp:231:15
    #46 0x7f35193436fa in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1115:52
    #47 0x7f3519345d51 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1317:15
    #48 0x7f35193274a6 in HandleEvent src/obj-firefox/dist/include/mozilla/EventListenerManager.h:390:5
    #49 0x7f35193274a6 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:425
    #50 0x7f3519325728 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:642:16
    #51 0x7f351932c180 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1164:11
    #52 0x7f35193333b6 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) src/dom/events/EventDispatcher.cpp
    #53 0x7f3515aa7a04 in nsINode::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) src/dom/base/nsINode.cpp:1141:5
    #54 0x7f351935772a in mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&) src/dom/events/EventTarget.cpp:205:13
    #55 0x7f35192a2013 in mozilla::AsyncEventDispatcher::Run() src/dom/events/AsyncEventDispatcher.cpp:72:12
    #56 0x7f3511679008 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1244:14
    #57 0x7f3511681dbd in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:530:10
    #58 0x7f35128f819f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #59 0x7f35127f3ebe in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #60 0x7f35127f3ebe in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #61 0x7f35127f3ebe in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #62 0x7f351b802af3 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
    #63 0x7f352024772e in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:961:22
    #64 0x7f35127f3ebe in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #65 0x7f35127f3ebe in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #66 0x7f35127f3ebe in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #67 0x7f3520246781 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:787:34
    #68 0x5566d24a9864 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #69 0x5566d24a9864 in main src/browser/app/nsBrowserApp.cpp:287
    #70 0x7f353534d82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #71 0x5566d23ceeec in _start (firefox+0x2deec)
Flags: in-testsuite?
Crash Signature: [@ nsWebBrowserFind::SetSelectionAndScroll ]
This crash is showing up on Nightly, too.
[Tracking Requested - why for this release]: new crash that is showing up on Nightly and is hopefully easy to fix, because it is a null deref.

Tyson, any chance you can do a regression window on this?

Judging by Nightly, the regression range is somewhere in: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=204cda7581188cfc8c8ef11dce4680dadf2b43bb&tochange=5c66354bff282452a6f1a3c911fa8756b6e752af
Flags: needinfo?(twsmith)
I do notice that nsWebBrowserFind::FindNext() is in the call stack, which uses nsIDocShell::ENUMERATE_BACKWARDS, and bug 1505601 is in the regression range. That seems like maybe a more likely culprit than something related to configurability of properties on window, which is the other DOM-ish thing in the range.
FWIW, the change to FindNext() looks okay to me, so maybe that's just a red herring.
I don't have tools setup to do it, Ryan do you have some free cycles?
Flags: needinfo?(twsmith) → needinfo?(ryanvm)
I'm doing it.
Flags: needinfo?(ryanvm)
Thanks, Emilio.
Flags: needinfo?(emilio)
Bug 1505887 changed the behavior here from content calling into nsFind via
window.find(), by making the SetStart and SetEnd calls here fail instead of
succeed for NAC (like the text in textareas).

This patch makes us handle that error gracefully moving on to the next match,
instead of trying to preserve the previous behavior.

This means that we'll fail to highlight textarea content and such from
window.find, which Chromium does, looks like. Though Chromium doesn't expose
the ranges as selection either. In any case I don't think that this is a very
common thing given bugs like bug 1442466, which this bug fixes.

I haven't found anything close to a spec for what window.find() should do... If
we decide to go with this patch then I'll add a crashtest for this and a test
for bug 1442466 as well. Otherwise I'll add a way to skip the security check in
nsFind somehow for NAC, or relax the security restrictions in SetStart /
SetEnd, I guess.
Blocks: 1442466
Please make sure this test gets landed too (assuming it's reliable enough to do so).
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aaa880bc536e
Properly handle errors in nsFind when returning a range. r=bzbarsky
https://hg.mozilla.org/mozilla-central/rev/aaa880bc536e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Please nominate this for Beta uplift when you get a chance.
Flags: needinfo?(emilio)
Flags: in-testsuite?
Flags: in-testsuite+
Comment on attachment 9030096 [details]
Bug 1510485 - Properly handle errors in nsFind when returning a range.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505887

User impact if declined: Crashes

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: none

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Trivial error-handling patch.

String changes made/needed: none
Flags: needinfo?(emilio)
Attachment #9030096 - Flags: approval-mozilla-beta?
Comment on attachment 9030096 [details]
Bug 1510485 - Properly handle errors in nsFind when returning a range.

[Triage Comment]
Fixes crashes caused by bug 1505887. Approved for 65.0b5.
Attachment #9030096 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: