Bug 77659 – NULL ptr in WebCore::Range::insertNode in SVG documents

• Status: REOPENED
• Component: DOM

[Berend-Jan Wever]

http://code.google.com/p/chromium/issues/detail?id=112483

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=17719940

Uploader: [e-mail]

Crash Type: UNKNOWN
Crash Address: 0x000000000026
Crash State:
  - crash stack -
  WebCore::Range::insertNode
  WebCore::Range::surroundContents
  WebCore::RangeInternal::surroundContentsCallback
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=108839:108881

Minimized Testcase (1.00 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96_OfO0wz1hUVMs3ZMqNXv62puXdfpj7QJ_eph_nf5GPfqUjK3C51OEXUKkL4jXtK8f2t9p4a5H6bvc-ESsANfSjuaCliUgG8iPBI3108sXgLqoTIOQWa7D19HOsNTwciA1mMlcltMsKjQjil0G-UwuO4zxOw

[Eric Seidel (no email)]

This is an insufficient report.  Linking to a locked-away application isn't very helpful.

Please fix your tools to file better bugs.

(Also, looking at the stacktrace, the stacktrace looks bogus.)

[Eric Seidel (no email)]

Sorry for the odd CC's, I started to CC Darin and Ryosuke, as they also know about range issues, but after finally getting to the stack trace, the trace looked bogus.

If the tool were public (not requiring a google account), it might make more sense.

[Berend-Jan Wever]

Created attachment 125158 [details]
Repro

Sorry for the confusion; I assumed that that link would be publicly accessible. The repro is attached.

<svg xmlns="http://www.w3.org/2000/svg">
<script>
<![CDATA[
 af =[], i = 0;
 function main(){af[i++% af.length]()}
  window._Document_0=document;
  window._Window_0=window;
  _Selection_0=window._Window_0.getSelection();
  af.push(function (){
    try{window._ProcessingInstruction_1=window._Document_0.createProcessingInstruction("x","x")}catch(e){cole.log(e)};
    try{window._Range_0=window._Selection_0.getRangeAt(9223372036854775804)}catch(e){cons(e)};
  })
  af.push(function (){
    try{window._Range_0.surroundContents(window._ProcessingInstruction_1)}catch(e){conog(e)};
  })
  af.push(function (){
    try{window._Selection_0.setBaseAndExtent(window.ocessingInstr0,00032768,_Document_0,47412)}catch(e){cole.log(e)};
    try{window._Range_0.detach()}catch(e){cons(e)};
  })
  document.addEventListener("DOMSubtreeModified",main,false);
  setInterval(main, 100);
]]>
</script>

id:             chrome.dll!WebCore::Range::insertNode ReadAV@NULL (a774923561bd78822366477b21073c62)
description:    Attempt to read from unallocated NULL pointer+0x14 in chrome.dll!WebCore::Range::insertNode
application:    Chromium 18.0.1011.0
stack:          chrome.dll!WebCore::Range::insertNode
                chrome.dll!WebCore::Range::surroundContents
                chrome.dll!WebCore::RangeInternal::surroundContentsCallback
                chrome.dll!v8::internal::HandleApiCallHelper<...>
                chrome.dll!v8::internal::Builtin_HandleApiCall
                chrome.dll!v8::internal::Invoke
                chrome.dll!v8::internal::Execution::Call
                ...

[Berend-Jan Wever]

See comment #2 and comment #3

[Elliott Sprehn]

Still crashes a year later which is sad:

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x0000000101ccab20 WebCore::Range::insertNode(WTF::PassRefPtr<WebCore::Node>, int&) + 704
1   com.apple.WebCore             	0x0000000101ccbe34 WebCore::Range::surroundContents(WTF::PassRefPtr<WebCore::Node>, int&) + 596
2   com.apple.WebCore             	0x0000000101aba607 WebCore::jsRangePrototypeFunctionSurroundContents(JSC::ExecState*) + 183
3   ???                           	0x0000309ca7a01265 0 + 53449385316965
4   com.apple.JavaScriptCore      	0x00000001010de90f JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1007
5   com.apple.JavaScriptCore      	0x000000010102cd15 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 69
6   com.apple.WebCore             	0x0000000101e3f7d4 WebCore::ScheduledAction::executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue, WebCore::ScriptExecutionContext*) + 516
7   com.apple.WebCore             	0x0000000101e3f3dc WebCore::ScheduledAction::execute(WebCore::Document*) + 156
8   com.apple.WebCore             	0x0000000101670683 WebCore::DOMTimer::fired() + 275
9   com.apple.WebCore             	0x0000000101feb384 WebCore::ThreadTimers::sharedTimerFiredInternal() + 148
10  com.apple.WebCore             	0x0000000101e7f993 WebCore::timerFired(__CFRunLoopTimer*, void*) + 51
11  com.apple.CoreFoundation      	0x00007fff85dccbb8 __CFRunLoopRun + 6488
12  com.apple.CoreFoundation      	0x00007fff85dcad8f CFRunLoopRunSpecific + 575
13  com.apple.HIToolbox           	0x00007fff88b787ee RunCurrentEventLoopInMode + 333
14  com.apple.HIToolbox           	0x00007fff88b785f3 ReceiveNextEventCommon + 310
15  com.apple.HIToolbox           	0x00007fff88b784ac BlockUntilNextEventMatchingListInMode + 59
16  com.apple.AppKit              	0x00007fff843ceeb2 _DPSNextEvent + 708
17  com.apple.AppKit              	0x00007fff843ce801 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 155
18  com.apple.AppKit              	0x00007fff8439468f -[NSApplication run] + 395
19  com.apple.WebCore             	0x0000000101e3af03 WebCore::RunLoop::run() + 67
20  com.apple.WebKit2             	0x00000001002e6165 WebKit::WebProcessMain(WebKit::CommandLine const&) + 1105
21  com.apple.WebKit2             	0x000000010029a459 WebKitMain + 311
22  com.apple.WebProcess          	0x0000000100000e5e main + 214
23  com.apple.WebProcess          	0x0000000100000d80 start + 52

[Elliott Sprehn]

Created attachment 192672 [details]
Crash Reduction

This will crash your tab immediately. The bug is apparently related to empty ranges and DOMSubtreeModified events.

[Elliott Sprehn]

So this looks like Range isn't Mutation Event safe. There's lots of code in there that does insertChild or removeChild and doesn't verify the state afterwards so calling range.detach() in the DOMSubtreeModified event cleared the Range internal state and then we crash when m_start.container() or m_end.container() is null.

We should fix this to not crash, but I think the real lesson is that mutation events suck. :/

[Ojan Vafai]

(In reply to comment #7)
> We should fix this to not crash, but I think the real lesson is that mutation events suck. :/

Sadly, even without mutation events, there are sync events that happen during appendChild/removeChild. Actually, focus (from autofocus) and blur (when removing the focused node) are the only ones I know of. I wonder if we could get away with making these async, or even if not async, make them fire at the end of the relevant operation (e.g. at the end of Range::insertNode by using an EventQueueScope) or at microtask time.

In either case, that's a whole separate project. In the short-term, I agree that we just need to harden this code against mutations.