432025 - Crash [@ CallQueryInterface<nsIDOMElement, nsIContent>] with contenteditable and execCommand contentReadOnly

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build within 100ms. This regressed between 2007-11-13 and 2007-11-14:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-11-13+04&maxdate=2007-11-14+09&cvsroot=%2Fcvsroot
so I think a regression from bug 207531.

http://crash-stats.mozilla.com/report/index/19aa9ea6-1927-11dd-8373-001cc45a2ce4?p=1
0  	xul.dll  	CallQueryInterface<nsIDOMElement, nsIContent>  	 nsCOMPtr.h:1691
1 	xul.dll 	FindSelectionRoot 	mozilla/editor/libeditor/text/nsEditorEventListeners.cpp:1084
2 	xul.dll 	nsTextEditorFocusListener::Focus 	mozilla/editor/libeditor/text/nsEditorEventListeners.cpp:1142
3 	xul.dll 	nsEventListenerManager::HandleEvent 	mozilla/content/events/src/nsEventListenerManager.cpp:1181
4 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	mozilla/content/events/src/nsEventDispatcher.cpp:241
5 	xul.dll 	nsEventDispatcher::Dispatch 	mozilla/content/events/src/nsEventDispatcher.cpp:483
6 	xul.dll 	nsEventStateManager::SendFocusBlur 	mozilla/content/events/src/nsEventStateManager.cpp:4687
7 	xul.dll 	nsEventStateManager::SetContentState 	mozilla/content/events/src/nsEventStateManager.cpp:4243
8 	xul.dll 	nsGenericElement::SetFocus 	mozilla/content/base/src/nsGenericElement.cpp:2539
9 	xul.dll 	nsGenericHTMLElement::SetElementFocus 	mozilla/content/html/content/src/nsGenericHTMLElement.cpp:3074
10 	xul.dll 	nsHTMLInputElement::Focus 	mozilla/content/html/content/src/nsHTMLSelectElement.cpp:1247
11 	xul.dll 	nsGenericHTMLElementTearoff::Focus 	mozilla/content/html/content/src/nsGenericHTMLElement.cpp:197
12 	xul.dll 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
13 	xul.dll 	XPCWrappedNative::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2369

The stacktrace looks rather similar to the one in bug 403965, so perhaps this is a security problem too? Marking security sensitive too.

Comment 1

[Bernd]

this triggers first:

###!!! ASSERTION: bad action nesting!: 'mActionNesting>0', file d:/moz_src/mozil
la/editor/libeditor/html/nsHTMLEditRules.cpp, line 387

and then 

###!!! ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush == nsContentU
tils::IsSafeToRunScript()', file d:/moz_src/mozilla/layout/base/nsPresShell.cpp,
 line 4505

Comment 2

[Bob Clary [:bc] (inactive)]

(In reply to comment #1)

> ###!!! ASSERTION: Someone forgot to block scripts: 'aIsSafeToFlush ==
> nsContentU
> tils::IsSafeToRunScript()', file

The stack to this is similar to that in bug 434790 

Comment 3

[Martijn Wargers (dead)]

Still crashes using current trunk build.

Comment 4

[Lucas Adamski [:ladamski]]

The last assertion I see is: ASSERTION: null parameter: 'aSource', file ../../../../dist/include/xpcom/

Because, indeed, aSource == 0x0.

Exception by nsISupportsUtils.h:202: return aSource->QueryInterface(....), so it looks like a null deref to me. 

Further up the stack it looks like we are getting a null rootElement around nsEditorEventListeners.cpp:1044

But I'd feel better if someone who knows the code could take a closer look to make sure rootElement couldn't be a bogus address than 0.

Comment 5

[Olli Pettay [:smaug]]

this is null pointer crash. patch coming

Comment 6

[Olli Pettay [:smaug]]

Attachment: proposed patch

Adds a null check and removes code which could never be executed (because CallQueryInterface would have crashed before that code).

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 353245 [details] [diff] [review]
proposed patch

>diff --git a/editor/libeditor/text/nsEditorEventListeners.cpp b/editor/libeditor/text/nsEditorEventListeners.cpp

>+    }

New line.

>     CallQueryInterface(rootElement, &root);

New line.

>     return root;

Comment 8

[Olli Pettay [:smaug]]

Attachment: with newlines

Comment 9

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081229 Minefield/3.2a1pre