Crash in mozilla::EditorEventListener::MouseClick

RESOLVED FIXED in Firefox 52

Status

()

P2
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: philipp, Assigned: masayuki)

Tracking

({crash, csectype-nullptr, testcase-wanted})

46 Branch
mozilla54
All
Windows 7
crash, csectype-nullptr, testcase-wanted
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox49 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 fixed, firefox53 fixed, firefox54 fixed)

Details

(Whiteboard: [sg:dos], crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-59b189b4-3f80-413d-a89b-1dbf42161105.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::EditorEventListener::MouseClick(nsIDOMMouseEvent*) 	editor/libeditor/EditorEventListener.cpp:673
1 	xul.dll 	mozilla::EditorEventListener::HandleEvent(nsIDOMEvent*) 	editor/libeditor/EditorEventListener.cpp:453
2 	xul.dll 	mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) 	dom/events/EventListenerManager.cpp:1133
3 	xul.dll 	mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) 	dom/events/EventListenerManager.cpp:1286
4 	xul.dll 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventDispatcher.cpp:357
5 	xul.dll 	mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) 	dom/events/EventDispatcher.cpp:710
6 	mozglue.dll 	mozilla::TimeStamp::Now(bool) 	mozglue/misc/TimeStamp_windows.cpp:529
7 	xul.dll 	nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayFallibleAllocator>(unsigned int, unsigned int) 	obj-firefox/dist/include/nsTArray-inl.h:169
8 	xul.dll 	nsCOMPtr_base::assign_with_AddRef(nsISupports*) 	xpcom/glue/nsCOMPtr.cpp:52
9 	xul.dll 	PresShell::HandleEventWithTarget(mozilla::WidgetEvent*, nsIFrame*, nsIContent*, nsEventStatus*) 	layout/base/nsPresShell.cpp:8103
10 	xul.dll 	mozilla::EventStateManager::CheckForAndDispatchClick(mozilla::WidgetMouseEvent*, nsEventStatus*) 	dom/events/EventStateManager.cpp:4642
11 	xul.dll 	mozilla::EventStateManager::PostHandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsEventStatus*) 	dom/events/EventStateManager.cpp:3143

this is a low level crash on windows 7 and above.

some of the user comments indicates that this is happening when they are using alternative input devices like a pen or the win10 touch keyboard.

many comments also seem to suggest that it is happening repeatedly on certain sites:
*keeps crashing at credit union site
https://crash-stats.mozilla.com/report/index/5ffe2bcc-5e34-44c7-a6cd-59a052160531
*same problem- crashing at credit union site
https://crash-stats.mozilla.com/report/index/14596ae9-0d33-4a09-a5f5-dfd782160606
*This is about the 4th or 5th time today I've had a crash. Starting to suspect it's the novlr.org website but submitting anyway in case it's something else.
https://crash-stats.mozilla.com/report/index/656d9b3d-bf35-4181-b2fa-fc9c62160624
*This is getting ridiculous. EVERY time I try to transfer money the site crashes. What gives?
https://crash-stats.mozilla.com/report/index/b990efab-6ed1-4212-8550-1888e2160803
*it appears this happens once i try to put in a dollar amount on my banking site. This has happened four times this morning..kind of worried.
https://crash-stats.mozilla.com/report/index/36d1a2fa-11f9-440c-9c81-9621e2160825
*What is wrong with this site. It continually crashes!!!!!!!!!
https://crash-stats.mozilla.com/report/index/744d035a-dfda-407a-992a-06f552160926
*Fourth crash. Happens every time I go on this site.
https://crash-stats.mozilla.com/report/index/b34d078d-87bf-41b5-9242-4cf252161028

Updated

2 years ago
Priority: -- → P2
Keywords: csectype-nullptr, testcase-wanted
Whiteboard: [sg:dos]
Why is mEditorBase null even if we already refcounted?
Although, I still don't have any idea to reproduce this, we should fix this and uplift because this bug will be "hidden" by another security fix but it won't be uplifted.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8831642 [details]
Bug 1315450 EditorEventListener::MouseClick() should check if mEditorBase is available before calling its ForceCompositionEnd()

https://reviewboard.mozilla.org/r/108192/#review109224
Attachment #8831642 - Flags: review?(bugs) → review+

Comment 6

2 years ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/f5e2e02ed7d2
EditorEventListener::MouseClick() should check if mEditorBase is available before calling its ForceCompositionEnd() r=smaug

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f5e2e02ed7d2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8831642 [details]
Bug 1315450 EditorEventListener::MouseClick() should check if mEditorBase is available before calling its ForceCompositionEnd()

Approval Request Comment
[Feature/Bug causing the regression]: Perhaps, ancient crash bug.
[User impact if declined]: Meets this crash when editor has gone at clicking in editor.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Still not sure how to reproduce this crash.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: No.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Because just adding nullptr check of mEditorBase before calling its method.
[String changes made/needed]: No.

FYI: This crash will be "hidden" by fixing bug 1314053 but the fix won't be uplifted due to risk management. So, I'd like to uplift only the fixes of crash safely.
Attachment #8831642 - Flags: approval-mozilla-beta?
Attachment #8831642 - Flags: approval-mozilla-aurora?
Comment on attachment 8831642 [details]
Bug 1315450 EditorEventListener::MouseClick() should check if mEditorBase is available before calling its ForceCompositionEnd()

avoid null dereference in editor, aurora53+, beta52+
Attachment #8831642 - Flags: approval-mozilla-beta?
Attachment #8831642 - Flags: approval-mozilla-beta+
Attachment #8831642 - Flags: approval-mozilla-aurora?
Attachment #8831642 - Flags: approval-mozilla-aurora+

Comment 10

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/e388fdc06380
status-firefox53: --- → fixed

Comment 11

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/91fad2ff9219
status-firefox52: affected → fixed
Flags: qe-verify-
Too late for 51. Mark 51 won't fix.
status-firefox50: affected → wontfix
status-firefox51: affected → wontfix
You need to log in before you can comment on or make changes to this bug.