481139 - Crash [@ nsWSRunObject::GetNextWSNode] with contenteditable, changing root

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

I've been hitting this a lot lately.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(I tested this on latest-trunk mozilla-central nightly build on WinXP SP3)

Turning security-sensitive and nominating blocking1.9.1? just-in-case, due to
!exploitable PROBABLY_EXPLOITABLE result.


0:000> !exploitable -v
HostMachine\HostUser
Executing Processor Architecture is x86
Debuggee is in User Mode
Debuggee is a live user mode debugging session on the local machine
Event Type: Exception
*** WARNING: Unable to verify checksum for C:\Documents and Settings\Administrator\Desktop\firefox\js3250.dll
Exception Faulting Address: 0x0
First Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005)
Exception Sub-Type: Read Access Violation

Faulting Instruction:105f2668 mov edx,dword ptr [eax]

Basic Block:
    105f2668 mov edx,dword ptr [eax]
       Tainted Input Operands: eax
    105f266a push ecx
    105f266b mov ecx,eax
       Tainted Input Operands: eax
    105f266d call dword ptr [edx+44h]
       Tainted Input Operands: ecx, edx

Exception Hash (Major/Minor): 0x195a0d66.0x53611e5d

Stack Trace:
xul!nsWSRunObject::GetNextWSNode+0x7f
xul!nsWSRunObject::GetNextWSNode+0x33
xul!nsWSRunObject::GetWSNodes+0x4de
xul!nsWSRunObject::nsWSRunObject+0x7b
xul!nsHTMLEditor::NormalizeEOLInsertPosition+0x35
xul!nsHTMLEditor::InsertElementAtSelection+0x1cf
xul!nsInsertTagCommand::DoCommand+0xbc
xul!nsControllerCommandTable::DoCommand+0x48
xul!nsBaseCommandController::DoCommand+0x64
xul!nsCommandManager::DoCommand+0x81
xul!nsHTMLDocument::ExecCommand+0x176
xul!NS_InvokeByIndex_P+0x27
xul!XPCWrappedNative::CallMethod+0x4fb
js3250!js_DefineNativeProperty+0x73d
js3250!js_DefineProperty+0x34
js3250!JS_DefinePropertyById+0x2f
xul!DefinePropertyIfFound+0x34d
xul!XPC_WN_CallMethod+0x114
js3250!js_Invoke+0x43b
js3250!js_Interpret+0x31d0
js3250!js_Invoke+0x289
js3250!js_InternalInvoke+0x102
js3250!JS_CallFunctionValue+0x27
xul!nsJSContext::CallEventHandler+0x162
xul!nsJSEventListener::HandleEvent+0x230
xul!nsEventListenerManager::HandleEventSubType+0x6b
xul!nsEventListenerManager::HandleEvent+0x222
xul!nsEventTargetChainItem::HandleEvent+0xb4
xul!nsEventTargetChainItem::HandleEventTargetChain+0x1eb
xul!nsEventDispatcher::Dispatch+0x4f5
xul!DocumentViewerImpl::LoadComplete+0x10c
xul!nsDocShell::EndPageLoad+0x63
xul!nsWebShell::EndPageLoad+0xa9
xul!nsDocShell::OnStateChange+0x83
xul!nsDocLoader::FireOnStateChange+0x123
xul!nsDocLoader::doStopDocumentLoad+0x1c
xul!nsDocLoader::DocLoaderIsEmpty+0x130
xul!nsDocLoader::OnStopRequest+0xc8
xul!nsLoadGroup::RemoveRequest+0xb8
xul!nsDocument::DoUnblockOnload+0x3b
Instruction Address: 0x105f2668

Description: Data from Faulting Address controls Code Flow
Short Description: TaintedDataControlsCodeFlow
Exploitability Classification: PROBABLY_EXPLOITABLE
Recommended Bug Title: Probably Exploitable - Data from Faulting Address controls Code Flow starting at xul!nsWSRunObject::GetNextWSNode+0x7f (Hash=0x195a0d66.0x53611e5d)

The data from the faulting address is later used as the target for a branch.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: null check

After thinking few options, null check is really the safest here.
Shouldn't regress anything, but does fix the crash.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

aStartNode is document here.

Comment 4

[Peter Van der Beken [:peterv]]

Would it work to QI to nsINode?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I was thinking that, but it wouldn't be as safe as this fix, since it would 
introduce new behavior in some cases.
This is just a null pointer crash, but perhaps we want this at least for
1.9.1, so I'd like to keep changes at minimum

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 369877 [details] [diff] [review]
null check

Please file a follow-up bug to QI to nsINode and add a comment here that refers to that bug.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Bug 487796

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/1a8349d0cff8