Closed Bug 1057677 Opened 6 years ago Closed 6 years ago

Crash by null pointer read in nsTextEditUtils::IsBreak

Categories

(Core :: DOM: Editor, defect, critical)

32 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla34
Tracking Status
firefox31 --- unaffected
firefox32 - wontfix
firefox33 + fixed
firefox34 + fixed

People

(Reporter: loobenyang, Assigned: ayg)

References

Details

(Keywords: crash, regression, reproducible)

Crash Data

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 (Beta/Release)
Build ID: 20140822030201

Steps to reproduce:

Open the reproduction test case in Firefox.

Reproduction case:

<html><body></body><script>
document.designMode = "on";
var hrElem = document.createElement("HR");
var select = window.getSelection(); 
document.body.appendChild(hrElem); 
select.collapse(hrElem,0); 
document.execCommand("InsertHTML", false, "<div>foo</div><div>bar</div>"); 
</script>
</html>


Firefox version:  34.0a1 (2014-08-22), nightly official build
Operating System:  Windows 8, 64 bit



Actual results:

Firefox crashed by null pointer read in nsTextEditUtils::IsBreak :

(2200.379c): Access violation - code c0000005 (!!! second chance !!!)
eax=0070e058 ebx=04412784 ecx=00000000 edx=00000000 esi=00000000 edi=184dde20
eip=0335b9b6 esp=0070df90 ebp=0070df90 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
xul!nsTextEditUtils::IsBreak+0x6:
0335b9b6 8b4118          mov     eax,dword ptr [ecx+18h] ds:002b:00000018=????????
0:000> !analyze -v
*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Windows\SysWOW64\SogouPy.ime - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Windows\SysWOW64\atidxx32.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\360\360Safe\safemon\urlproc.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\360\360Safe\360NetBase.dll - 

FAULTING_IP: 
xul!nsTextEditUtils::IsBreak+6 [c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\editor\libeditor\nstexteditutils.cpp @ 46]
0335b9b6 8b4118          mov     eax,dword ptr [ecx+18h]

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 0335b9b6 (xul!nsTextEditUtils::IsBreak+0x00000006)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000000
   Parameter[1]: 00000018
Attempt to read from address 00000018

CONTEXT:  00000000 -- (.cxr 0x0;r)
eax=0070e058 ebx=04412784 ecx=00000000 edx=00000000 esi=00000000 edi=184dde20
eip=0335b9b6 esp=0070df90 ebp=0070df90 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
xul!nsTextEditUtils::IsBreak+0x6:
0335b9b6 8b4118          mov     eax,dword ptr [ecx+18h] ds:002b:00000018=????????

FAULTING_THREAD:  0000379c

DEFAULT_BUCKET_ID:  NULL_CLASS_PTR_READ

PROCESS_NAME:  firefox.exe

ERROR_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%08lx referenced memory at 0x%08lx. The memory could not be %s.

EXCEPTION_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%08lx referenced memory at 0x%08lx. The memory could not be %s.

EXCEPTION_PARAMETER1:  00000000

EXCEPTION_PARAMETER2:  00000018

READ_ADDRESS:  00000018 

FOLLOWUP_IP: 
xul!nsTextEditUtils::IsBreak+6 [c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\editor\libeditor\nstexteditutils.cpp @ 46]
0335b9b6 8b4118          mov     eax,dword ptr [ecx+18h]

NTGLOBALFLAG:  0

APPLICATION_VERIFIER_FLAGS:  0

APP:  firefox.exe

ANALYSIS_VERSION: 6.3.9600.17029 (debuggers(dbg).140219-1702) x86fre

PRIMARY_PROBLEM_CLASS:  NULL_CLASS_PTR_READ

BUGCHECK_STR:  APPLICATION_FAULT_NULL_CLASS_PTR_READ

LAST_CONTROL_TRANSFER:  from 0333e19c to 0335b9b6

STACK_TEXT:  
0070df90 0333e19c 00000000 1daa2e80 03f0a178 xul!nsTextEditUtils::IsBreak+0x6
0070e14c 03346141 0070e1c4 00000000 04425080 xul!nsHTMLEditor::DoInsertHTMLWithContext+0x732
0070e17c 0330a9d3 184ddedc 0070e1c4 04425080 xul!nsHTMLEditor::InsertHTMLWithContext+0x2d
0070e1ac 0336abc9 184ddedc 0070e1c4 1da338cc xul!nsHTMLEditor::InsertHTML+0x1f
0070e25c 0348def2 1cf01c18 0070e370 1daa2e80 xul!nsInsertHTMLCommand::DoCommandParams+0xe3
0070e27c 0348d876 1dafec80 0070e370 1daa2e80 xul!nsControllerCommandTable::DoCommandParams+0x2f
0070e2a4 0348e363 184dde20 0070e370 1daa2e80 xul!nsBaseCommandController::DoCommandWithParams+0x62
0070e2c0 03d49004 1dafedc0 0070e370 1daa2e80 xul!nsCommandManager::DoCommand+0x55
0070e3b4 03d4d7f9 1c7cb800 0070e490 09a83000 xul!nsHTMLDocument::ExecCommand+0x31f
0070e528 02e4948a 184a0780 0070e554 1c7cb800 xul!mozilla::dom::HTMLDocumentBinding::execCommand+0x170
0070e568 0f3c95da 184a0780 00000003 17b4daf0 xul!mozilla::dom::GenericBindingMethod+0xaa
0070e788 0f3c7732 184a0780 09a83070 00000003 mozjs!js::Invoke+0x20a
0070ed08 0f3c93b2 184a0780 184a0780 0070edb8 mozjs!Interpret+0x3ac2
0070ed24 0f3c9a47 184a0780 0070ed3c 00000000 mozjs!js::RunScript+0x162
0070ed84 0f3c9b34 184a0780 0070ee6c 17b4a310 mozjs!js::ExecuteKernel+0xb7
0070edc0 0f30d4b7 184a0780 0070ee6c 17b4a310 mozjs!js::Execute+0xc4
0070ee70 0f30d82a 0070eed4 0070f08c 0070ef34 mozjs!Evaluate+0x107
0070ee88 02c757d8 184a0780 0070eed4 0070ef98 mozjs!JS::Evaluate+0x1a
0070ef00 02e3703a 0070f08c 0070ef88 0070ef34 xul!nsJSUtils::EvaluateString+0x288
0070ef38 02dd2f2a 0070f08c 0070ef88 0070ef98 xul!nsJSUtils::EvaluateString+0x38
0070f070 02dc605a 1da301a0 0070f08c 00000000 xul!nsScriptLoader::EvaluateScript+0x1b2
0070f168 02dda967 00000000 1d9841dc 1d95a4c0 xul!nsScriptLoader::ProcessRequest+0x1da
0070f2d8 02dda774 1d95a4c0 18751060 1d9841dc xul!nsScriptLoader::ProcessScriptElement+0x1d0
0070f300 02e557a4 18751060 1da90af8 02e25f70 xul!nsScriptElement::MaybeProcessScript+0x101
0070f30c 02e25f70 0000000d 1d9841dc 0070f348 xul!nsIScriptElement::AttemptToExecute+0xd
0070f31c 02d70cf1 1d984190 00e49000 00000000 xul!nsHtml5TreeOpExecutor::RunScript+0x4d
0070f348 02e1ae9e 00e20160 0070f3f4 02cdb73d xul!nsHtml5TreeOpExecutor::RunFlushLoop+0x1f1
0070f354 02cdb73d 1d94e470 00e2a0c0 00e0eb30 xul!nsHtml5ExecutorFlusher::Run+0x19
0070f3f4 02cbdfed 01e20160 00000918 0070f413 xul!nsThread::ProcessNextEvent+0x2cd
0070f408 02fa829b 01e20160 00000000 00e2a0c0 xul!NS_ProcessNextEvent+0x2d
0070f434 02f9b42f 01e2a0c0 e7a4bfd6 00e20160 xul!mozilla::ipc::MessagePump::Run+0x46
0070f46c 02f9ba3e 096185f0 00000001 02f98500 xul!MessageLoop::RunHandler+0x51
0070f48c 02ee7697 80000000 00ed1bc0 0070f4a8 xul!MessageLoop::Run+0x19
0070f49c 02f00fc8 096185f0 0070f4b8 0354427a xul!nsBaseAppShell::Run+0x2e
0070f4a8 0354427a 096185f0 0070f6bd 0070f58c xul!nsAppShell::Run+0x1b
0070f4b8 02e70329 00ed1bc0 658b13e5 0070f5cc xul!nsAppStartup::Run+0x20
0070f58c 02ecf246 0070f5cc 0070f6f4 0070f5cc xul!XREMain::XRE_mainRun+0x514
0070f5ac 02eeeb55 0070f5cc 00000000 008067b8 xul!XREMain::XRE_main+0xf0
0070f6c4 00fa1753 00000001 008067b8 0070f6f4 xul!XRE_main+0x30
0070f858 00fa1a17 00000001 008067b8 00e1c100 firefox!do_main+0x2e1
0070f8ec 00fa1b2b 00000001 008067b8 00000001 firefox!NS_internal_main+0x11d
0070f920 00fa244e 00000000 008029f8 008028e0 firefox!wmain+0xf9
0070f964 754486e3 fee2f000 0070f9b4 778ebe99 firefox!__tmainCRTStartup+0x122
0070f970 778ebe99 fee2f000 955b3a8e 00000000 KERNEL32!BaseThreadInitThunk+0xe
0070f9b4 778ebe6c 00fa256f fee2f000 ffffffff ntdll!__RtlUserThreadStart+0x72
0070f9cc 00000000 00fa256f fee2f000 00000000 ntdll!_RtlUserThreadStart+0x1b


STACK_COMMAND:  .cxr 0x0 ; kb

FAULTING_SOURCE_LINE:  c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\editor\libeditor\nstexteditutils.cpp

FAULTING_SOURCE_FILE:  c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\editor\libeditor\nstexteditutils.cpp

FAULTING_SOURCE_LINE_NUMBER:  46

SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  xul!nsTextEditUtils::IsBreak+6

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: xul

IMAGE_NAME:  xul.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  53f73c88

FAILURE_BUCKET_ID:  NULL_CLASS_PTR_READ_c0000005_xul.dll!nsTextEditUtils::IsBreak

BUCKET_ID:  APPLICATION_FAULT_NULL_CLASS_PTR_READ_xul!nsTextEditUtils::IsBreak+6

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:null_class_ptr_read_c0000005_xul.dll!nstexteditutils::isbreak

FAILURE_ID_HASH:  {46822038-af82-6706-1974-b9984bdd03e7}

Followup: MachineOwner
---------




Expected results:

Should not crash.
It affects Firefox beta 32.0:

(19c4.3a5c): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=00b2e104 ebx=51ee1c60 ecx=00000000 edx=00000000 esi=00000000 edi=07d8eac0
eip=50e4b321 esp=00b2e044 ebp=00b2e1b0 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
xul!nsTextEditUtils::IsBreak+0x4:
50e4b321 8b4118          mov     eax,dword ptr [ecx+18h] ds:002b:00000018=????????
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

CR with FF34 on Win 7:
https://crash-stats.mozilla.com/report/index/b13d5efb-b451-4802-9cc1-4591c2140823

Reg range:
good=2014-05-06
bad=2014-05-07
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=81651ad5e43c&tochange=5bbc85136202

Suspected: One of bugs patched by Aryeh Gregor: bug 1003808, bug 1002429, bug 1003894
Severity: normal → critical
Status: UNCONFIRMED → NEW
Crash Signature: [@ nsTextEditUtils::IsBreak(nsINode*) ]
Component: Untriaged → Editor
Ever confirmed: true
Flags: needinfo?(ayg)
Keywords: crash, reproducible
Product: Firefox → Core
Version: 34 Branch → 32 Branch
In local build
Last Good: af3a7489a11f
First Bad: b090934d5f7f
Regressed by:
	b090934d5f7f	Aryeh Gregor — Bug 1003808 part 6 - Convert nsWSRunObject members to nsINode; r=ehsan
Blocks: 1003808
Ty!
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Flags: needinfo?(ayg)
Attached patch PatchSplinter Review
Well, this fixes the crash, but I'm not sure it should be necessary.  Is mEndReasonNode supposed to always be non-null?  nsWSRunObject::GetWSNodes() doesn't seem to necessarily do that.  In this case, it bails out over here:

http://hg.mozilla.org/mozilla-central/file/daa84204a11a/editor/libeditor/nsWSRunObject.cpp#l780

which happens because we hit the one failure case here:

http://hg.mozilla.org/mozilla-central/file/daa84204a11a/editor/libeditor/nsWSRunObject.cpp#l1121

It looks like a better way to fix this would be to make that function infallible, and have it return null in this case.  GetWSNodes() will deal with that happily.  Does that sound like a good idea to do in a followup, Ehsan?  I'm still submitting this patch because a) it's a good idea anyway, since there are probably other ways GetWSNodes() can fail; and b) this patch is safer for beta.
Attachment #8477978 - Flags: review?(ehsan)
Unfortunately, it's too late to take this fix in beta 32. I have tracked this for 33 and 34.
Comment on attachment 8477978 [details] [diff] [review]
Patch

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

Making this infallible sounds like a good idea, but let's do that in a follow-up as this needs to be backported to 33 and 34.  Thanks!

::: editor/libeditor/crashtests/1057677.html
@@ +3,5 @@
> +var hrElem = document.createElement("HR");
> +var select = window.getSelection(); 
> +document.body.appendChild(hrElem); 
> +select.collapse(hrElem,0); 
> +document.execCommand("InsertHTML", false, "<div>foo</div><div>bar</div>"); 

Nit: please remove the trailing spaces.
Attachment #8477978 - Flags: review?(ehsan) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/a6832b6e110d
Flags: in-testsuite+
OS: Windows 8 → All
Hardware: x86_64 → All
Can you please request branch approvals?  Thanks!
Flags: needinfo?(ayg)
https://hg.mozilla.org/mozilla-central/rev/a6832b6e110d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Comment on attachment 8477978 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1003808
[User impact if declined]: Crash that could be triggered by malicious web pages (I don't know if it's likely to occur in the wild)
[Describe test coverage new/current, TBPL]: Passed try, landed on central
[Risks and why]: Negligible risk -- just a null check
[String/UUID change made/needed]: None
Attachment #8477978 - Flags: approval-mozilla-aurora?
Flags: needinfo?(ayg)
Attachment #8477978 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.