44170 2010-08-18 06:21:16 -0700 HTML5 TreeBuilder ASSERTs on <a><svg><tr><input></a> 2010-08-21 20:07:15 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED InRadar P1 Major --- 43055 44129 44390 1 skylined tonyg abarth ddkilzer eric inferno sam tonyg oldest_to_newest 266072 0 64697 skylined 2010-08-18 06:21:16 -0700 Created attachment 64697 Repro The following repro causes memory corruption in Chromium: <body onload=" document.writeln('<a ><svg ><tr/s><input></a>'); document.open(); "> Crashes vary wildly, but most commonly I see a crash in WebCore::removeAllChildrenInContainer<WebCore::Node,WebCore::ContainerNode> 266140 1 inferno 2010-08-18 09:04:21 -0700 Adding security as product. 266143 2 eric 2010-08-18 09:08:44 -0700 I feel like this is identical to another one you filed this morning. Like bug 44172. These are probably all related to/duplicates of bug 43055. 266145 3 ddkilzer 2010-08-18 09:09:33 -0700 <rdar://problem/8324425> 266151 4 eric 2010-08-18 09:11:19 -0700 I'll be landing a fix for bug 43055 this afternoon, then I can look at some of these others. I suspect most of them will be dupes. :) 266157 5 inferno 2010-08-18 09:20:45 -0700 Eric, this bug reproduces with just <a><svg><tr><input></a> and pressing page reload. It hits this assert without reload. I am taking a closer look now, dont think it is related to 43055. ASSERT(!newChild->parent()); // Use appendChild if you need to handle reparenting. WebCore::ContainerNode::addChildCommon(WebCore::Node * newChild=0x08153780) Line 539 + 0x2b bytes WebCore::ContainerNode::parserAddChild(WTF::PassRefPtr<WebCore::Node> newChild={m_document=0x08219000 m_previous=0x00000000 m_next=0x00000000 ...}) Line 559 WebCore::HTMLConstructionSite::attachAtSite(const WebCore::HTMLConstructionSite::AttachmentSite & site={...}, WTF::PassRefPtr<WebCore::Node> prpChild=NULL) Line 133 WebCore::HTMLConstructionSite::fosterParent(WebCore::Node * node=0x08153780) Line 457 WebCore::HTMLTreeBuilder::callTheAdoptionAgency(WebCore::AtomicHTMLToken & token={...}) Line 1835 WebCore::HTMLTreeBuilder::processEndTagForInBody(WebCore::AtomicHTMLToken & token={...}) Line 2161 WebCore::HTMLTreeBuilder::processEndTag(WebCore::AtomicHTMLToken & token={...}) Line 2295 WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken & token={...}) Line 629 WebCore::HTMLTreeBuilder::processUsingSecondaryInsertionModeAndAdjustInsertionMode(WebCore::AtomicHTMLToken & token={...}) Line 2539 WebCore::HTMLTreeBuilder::processEndTag(WebCore::AtomicHTMLToken & token={...}) Line 2499 WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken & token={...}) Line 629 WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken & rawToken={...}) Line 612 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode mode=AllowYield) Line 196 WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode mode=AllowYield) Line 151 WebCore::HTMLDocumentParser::append(const WebCore::SegmentedString & source={...}) Line 282 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter * writer=0x0508b18c, const char * data=0x00000000, int length=0, bool shouldFlush=true) Line 55 + 0x1f bytes WebCore::DocumentWriter::addData(const char * str=0x00000000, int len=0, bool flush=true) Line 200 + 0x20 bytes WebCore::DocumentWriter::endIfNotLoadingMainResource() Line 221 WebCore::DocumentWriter::end() Line 207 WebCore::DocumentLoader::finishedLoading() Line 270 WebCore::FrameLoader::finishedLoading() Line 2160 266159 6 eric 2010-08-18 09:26:55 -0700 Sweet! A new test for the parser. No one has shipped this. I'm not sure this needs to be a security bug. Adam, Tony or I will have a patch landed this afternoon. 266162 7 eric 2010-08-18 09:30:28 -0700 Thanks again for the test case! This is obviously security related, but doesn't need to be marked sensitive since this code has not (and will not) ship as is. 266164 8 inferno 2010-08-18 09:37:18 -0700 Thanks Eric, Adam, Tony for taking care of it. Another security bug off our radar :) Marking it security helps to keep track, but since i am on the cc, i will update chromium side bug when it gets fixed - http://code.google.com/p/chromium/issues/detail?id=52581. 266171 9 eric 2010-08-18 09:46:40 -0700 I suspect we'll find a number of edge-cases like this in the new tree builder. They'll be simple to fix (unlike the old tree builder), but I expect there may be a bunch, so I'm glad you're fuzzing. 266329 10 tonyg 2010-08-18 13:05:37 -0700 Eric and I chatted about this briefly on IRC. I'll take a look. 266394 11 tonyg 2010-08-18 14:53:08 -0700 (In reply to comment #10) > Eric and I chatted about this briefly on IRC. I'll take a look. I'm releasing this bug for now so that I can focus on 44129 (which turned out to be more involved than I thought). Eric, if you don't get to this first, I'm happy to pick it up again. Here are my notes so far: TreeBuilder has this branch: if causesFosterParenting tree.fosterParent() else commonAncestor.appendChild() This case takes the fosterParent() route for the <svg> element which invokes attachAtSite() and calls parserAddChild(). That method doesn't handle reparenting. Changing parserAddChild() to appendChild() fixes the crash because it handles reparenting. However, I'm positive that is not the solution in light of our discussion of mutation events. Perhaps the answer is to teach parserAddChild() about reparenting or in some other way address the FIXME in attach() about attachAtSite() not handling foster parenting. 266837 12 tonyg 2010-08-19 11:09:06 -0700 It is looking like a single patch that reparents in AAA step 7 and uses parser* methods for insertBefore, removeChild, addChild will fix this and 44129. 267497 13 tonyg 2010-08-20 17:28:54 -0700 Crash fixed by r65769. The dom tree doesn't seem right. Filed spec bug here: http://www.w3.org/Bugs/Public/show_bug.cgi?id=10409 64697 2010-08-18 06:21:16 -0700 2010-08-18 06:21:16 -0700 Repro repro.html text/html 88 skylined PGJvZHkgb25sb2FkPSIKICBkb2N1bWVudC53cml0ZWxuKCc8YSA+PHN2ZyA+PHRyL3M+PGlucHV0 PjwvYT4nKTsKICBkb2N1bWVudC5vcGVuKCk7CiI+Cg==