Bug 64801 – Use after free in ReplacementFragment::removeUnrenderedNodes

• Status: RESOLVED
• Resolution: FIXED
• Component: Security

[Abhishek Arya]

found in my fuzzing + ASAN
http://code.google.com/p/chromium/issues/detail?id=89678

Chromium Revision : 92735
Webkit Revision : 91062

Reduced Testcase (run from LayoutTests/editing/pasteboard)
<input id="test" contenteditable="true"><script src="../editing.js">></script>
<script>
var e = document.getElementById("test");
var s = window.getSelection();

s.setPosition(e, 10500000000);
insertHTMLCommand("<noscript>baz");

</script>

==================================================================
HINT: if your stack trace looks short or garbled, use ASAN_OPTIONS=fast_unwind=0
==1708== ERROR: AddressSanitizer crashed on address 0x00007f8acbb7bd10 at pc 0x7f8b0e2b09a1 bp 0x7f8af55d0d30 sp 0x7f8af55d0bc0
READ of size 4 at 0x00007f8acbb7bd10 thread T12
    #0 0x7f8b0e2b09a1 in WebCore::ReplacementFragment::removeUnrenderedNodes(WebCore::Node*) 
    #1 0x7f8b0e2adc9d in WebCore::ReplacementFragment::ReplacementFragment(WebCore::Document*, WebCore::DocumentFragment*, bool, WebCore::VisibleSelection const&) 
    #2 0x7f8b0e2bba68 in WebCore::ReplaceSelectionCommand::doApply() 
    #3 0x7f8b0e1fe7eb in WebCore::EditCommand::apply() 
    #4 0x7f8b0e252454 in WebCore::executeInsertFragment(WebCore::Frame*, WTF::PassRefPtr<WebCore::DocumentFragment>) third_party/WebKit/Source/WebCore/editing/EditorCommand.cpp:0
    #5 0x7f8b0e24a82e in WebCore::executeInsertHTML(WebCore::Frame*, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) third_party/WebKit/Source/WebCore/editing/EditorCommand.cpp:0
    #6 0x7f8b0e246fd0 in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 
    #7 0x7f8b0d9356f1 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 
    #8 0x7f8b0d425b45 in WebCore::DocumentInternal::execCommandCallback(v8::Arguments const&) out/Release/obj/gen/webkit/bindings/V8DerivedSources16.cpp:0
    #9 0x7f8b0c541d9f in v8::internal::Builtin_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) v8/src/builtins.cc:0
    #10 0x7f8acf2f014e in  
0x00007f8acbb7bd10 is located 16 bytes inside of 88-byte region [0x00007f8acbb7bd00,0x00007f8acbb7bd58)
freed by thread T12 here:
    #1 0x7f8b0d8e7246 in void WebCore::removeAllChildrenInContainer<WebCore::Node, WebCore::ContainerNode>(WebCore::ContainerNode*) 
    #2 0x7f8b0d8e8792 in WebCore::ContainerNode::~ContainerNode() 
    #3 0x7f8b0da7e3e5 in non-virtual thunk to WebCore::HTMLElement::~HTMLElement() 
    #4 0x7f8b0e2b0973 in WebCore::ReplacementFragment::removeUnrenderedNodes(WebCore::Node*) 
    #5 0x7f8b0e2adc9d in WebCore::ReplacementFragment::ReplacementFragment(WebCore::Document*, WebCore::DocumentFragment*, bool, WebCore::VisibleSelection const&) 
    #6 0x7f8b0e2bba68 in WebCore::ReplaceSelectionCommand::doApply() 
    #7 0x7f8b0e1fe7eb in WebCore::EditCommand::apply() 
    #8 0x7f8b0e252454 in WebCore::executeInsertFragment(WebCore::Frame*, WTF::PassRefPtr<WebCore::DocumentFragment>) third_party/WebKit/Source/WebCore/editing/EditorCommand.cpp:0
    #9 0x7f8b0e24a82e in WebCore::executeInsertHTML(WebCore::Frame*, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) third_party/WebKit/Source/WebCore/editing/EditorCommand.cpp:0
    #10 0x7f8b0e246fd0 in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 
    #11 0x7f8b0d9356f1 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 
    #12 0x7f8b0d425b45 in WebCore::DocumentInternal::execCommandCallback(v8::Arguments const&) out/Release/obj/gen/webkit/bindings/V8DerivedSources16.cpp:0
    #13 0x7f8b0c541d9f in v8::internal::Builtin_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) v8/src/builtins.cc:0
    #14 0x7f8acf2f014e in  
    #15 0x7f8acf317d0f in  
previously allocated by thread T12 here:
    #1 0x7f8b0da39085 in WebCore::Text::create(WebCore::Document*, WTF::String const&) 
    #2 0x7f8b0da3d4a7 in WebCore::Text::createWithLengthLimit(WebCore::Document*, WTF::String const&, unsigned int, unsigned int) 
    #3 0x7f8b0dcd3e13 in WebCore::HTMLConstructionSite::insertTextNode(WTF::String const&) 
    #4 0x7f8b0dc59d44 in WebCore::HTMLTreeBuilder::processCharacterBuffer(WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer&) 
    #5 0x7f8b0dc4b804 in WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken&) 
    #6 0x7f8b0dc4b255 in WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken(WebCore::AtomicHTMLToken&) 
    #7 0x7f8b0dc4b0f0 in WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken&) 
    #8 0x7f8b0dc02d9e in WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) 
    #9 0x7f8b0dc042d4 in WebCore::HTMLDocumentParser::insert(WebCore::SegmentedString const&) 
    #10 0x7f8b0dc0657b in WebCore::HTMLDocumentParser::parseDocumentFragment(WTF::String const&, WebCore::DocumentFragment*, WebCore::Element*, WebCore::FragmentScriptingPermission) 
    #11 0x7f8b0d9638c4 in WebCore::Element::deprecatedCreateContextualFragment(WTF::String const&, WebCore::FragmentScriptingPermission) 
    #12 0x7f8b0daa136c in WebCore::HTMLElement::deprecatedCreateContextualFragment(WTF::String const&, WebCore::FragmentScriptingPermission) 
    #13 0x7f8b0e357e4e in WebCore::createFragmentFromMarkup(WebCore::Document*, WTF::String const&, WTF::String const&, WebCore::FragmentScriptingPermission) 
    #14 0x7f8b0e24a823 in WebCore::executeInsertHTML(WebCore::Frame*, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) third_party/WebKit/Source/WebCore/editing/EditorCommand.cpp:0
    #15 0x7f8b0e246fd0 in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 
Thread T12 created by T0 here:
    #1 0x7f8b0ba527b1 in base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, unsigned long*) base/threading/platform_thread_posix.cc:0
    #2 0x7f8b0ba5267a in base::PlatformThread::Create(unsigned long, base::PlatformThread::Delegate*, unsigned long*) 
    #3 0x7f8b0ba532b0 in base::Thread::StartWithOptions(base::Thread::Options const&) 
    #4 0x7f8b0f800e11 in BrowserRenderProcessHost::Init(bool) 
    #5 0x7f8b0f86e56f in RenderViewHost::CreateRenderView(std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > const&) 
    #6 0x7f8b0f916e77 in TabContents::CreateRenderViewForRenderManager(RenderViewHost*) 
    #7 0x7f8b0f9172bd in non-virtual thunk to TabContents::CreateRenderViewForRenderManager(RenderViewHost*) 
==1708== ABORTING
Stats: 0M malloced (0M for red zones) by 0 calls
Stats: 2M realloced by 6690 calls
Stats: 0M freed by 0 calls
Stats: 0M really freed by 0 calls
Stats: 0M (0 pages) mmaped in 0 calls
Stats: 48M of shadow memory allocated in 48 clusters
             (1M each, 0 low and 48 high)
Shadow byte and word:
  0x00001ff15976f7a2: fb
  0x00001ff15976f7a0: fb fb fb fb fb fb fb fb
More shadow bytes:
  0x00001ff15976f780: ff ff ff ff ff ff ff ff
  0x00001ff15976f788: ff ff ff ff ff ff ff ff
  0x00001ff15976f790: ff ff ff ff ff ff ff ff
  0x00001ff15976f798: ff ff ff ff ff ff ff ff
=>0x00001ff15976f7a0: fb fb fb fb fb fb fb fb
  0x00001ff15976f7a8: fb fb fb fb fb fb fb fb
  0x00001ff15976f7b0: fb fb fb fb fb fb fb fb
  0x00001ff15976f7b8: fb fb fb fb fb fb fb fb
  0x00001ff15976f7c0: ff ff ff ff ff ff ff ff


The problem is looks to due to the ugly iteration logic with raw pointers.

void ReplacementFragment::removeUnrenderedNodes(Node* holder)
{
    Vector<Node*> unrendered;
...........

    size_t n = unrendered.size();
    for (size_t i = 0; i < n; ++i)
        removeNode(unrendered[i]);

[Lucas Forschler]

<rdar://problem/9800184>

[Abhishek Arya]

Working on the fix.

[Abhishek Arya]

Created attachment 101335 [details]
Patch

[Darin Adler]

Comment on attachment 101335 [details]
Patch

Looks like the right fix to me.

[WebKit Review Bot]

Comment on attachment 101335 [details]
Patch

Clearing flags on attachment: 101335

Committed r91270: <http://trac.webkit.org/changeset/91270>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Lucas Forschler]

<rdar://problem/9809494>

[Ademar Reis]

Revision r91270 cherry-picked into qtwebkit-2.2 with commit 4e1a62f <http://gitorious.org/webkit/qtwebkit/commit/4e1a62f>