Created attachment 65416 [details] Viewing the attached SVG crashes Safari/Chrome Opening the attached SVG file causes crashes both in Safari and Google Chrome. Quick stack trace from OS X 10.6.5 (10H529), Safari Version 5.0.1 (6533.17.8) follows: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000 0x00007fff8714d312 in WebCore::updateContainerOffset () (gdb) bt #0 0x00007fff8714d312 in WebCore::updateContainerOffset () #1 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #2 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #3 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #4 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #5 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #6 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #7 0x00007fff8714d2d8 in WebCore::updateContainerOffset () #8 0x00007fff8714d0f0 in WebCore::SVGUseElement::updateContainerOffsets () #9 0x00007fff8714bf3a in WebCore::SVGUseElement::buildShadowAndInstanceTree () #10 0x00007fff8714b709 in WebCore::RenderSVGShadowTreeRootContainer::updateFromElement () #11 0x00007fff86bf07d1 in WebCore::ContainerNode::dispatchPostAttachCallbacks () #12 0x00007fff86ae25ff in WebCore::ContainerNode::resumePostAttachCallbacks () #13 0x00007fff86ae85ed in WebCore::Element::attach () #14 0x00007fff86c31be4 in WebCore::ContainerNode::appendChild () #15 0x00007fff86d62732 in WebCore::XMLTokenizer::insertErrorMessageBlock () #16 0x00007fff86bbdb68 in WebCore::XMLTokenizer::end () #17 0x00007fff86ad05f2 in WebCore::DocumentWriter::endIfNotLoadingMainResource () #18 0x00007fff86b899ab in WebCore::FrameLoader::finishedLoading () #19 0x00007fff86b898f0 in WebCore::MainResourceLoader::didFinishLoading () #20 0x00007fff8249938c in _NSURLConnectionDidFinishLoading () #21 0x00007fff80206646 in URLConnectionClient::_clientDidFinishLoading () #22 0x00007fff8026bd16 in URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload () #23 0x00007fff8026bf82 in URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload () #24 0x00007fff801f2cbf in URLConnectionClient::processEvents () #25 0x00007fff801f2a9c in MultiplexerSource::perform () #26 0x00007fff8179c5f1 in __CFRunLoopDoSources0 () #27 0x00007fff8179a7e9 in __CFRunLoopRun () #28 0x00007fff81799faf in CFRunLoopRunSpecific () #29 0x00007fff8503991a in RunCurrentEventLoopInMode () #30 0x00007fff8503971f in ReceiveNextEventCommon () #31 0x00007fff850395d8 in BlockUntilNextEventMatchingListInMode () #32 0x00007fff83ff3e64 in _DPSNextEvent () #33 0x00007fff83ff37a9 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #34 0x0000000100015938 in ?? () #35 0x00007fff83fb948b in -[NSApplication run] () #36 0x00007fff83fb21a8 in NSApplicationMain () #37 0x0000000100009804 in ?? () Does not appear to be exploitable.
Created attachment 77486 [details] Patch
Comment on attachment 77486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77486&action=review LGTM. But reviewing the patch gives following error: Can't expand. Unable to apply patch to tip of tree. Can you check if it is still possible to apply the patch to tot? And a short snippet left > LayoutTests/svg/dom/recursive-use-expected.txt:1 > +Survived the crash! Shouldn't it be. "Did not crash!"? IIRC we don't want a crash at all ;-)
Comment on attachment 77486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77486&action=review Good approach, r- for a few nitpicks: > LayoutTests/ChangeLog:7 > + Test case for recursive svg <use>. > + This test case passes if no crash occurs. > + https://bugs.webkit.org/show_bug.cgi?id=44610 This text should go below the link to the bug report, instead the bug title should be above the link: <bug title> https://bugs.webkit.org/... <bug description> > LayoutTests/ChangeLog:10 > + * svg/dom/recursive-use-expected.txt: Added. > + * svg/dom/recursive-use.svg: Added. I think the testcase should rather go into svg/custom/, we only use svg/dom/ for tests that access any DOM attribute/props. > WebCore/ChangeLog:9 > + We should check recursive <use> at the begining of > + WebCore::SVGUseElement::buildInstanceTree instead > + of at the end of it because the target element's > + children may cause infinite recursive <use>. > + https://bugs.webkit.org/show_bug.cgi?id=44610 This text should go below the link to the bug report, instead the bug title should be above the link: <bug title> https://bugs.webkit.org/... <bug description>
Created attachment 77531 [details] Revised patch
Comment on attachment 77531 [details] Revised patch View in context: https://bugs.webkit.org/attachment.cgi?id=77531&action=review > LayoutTests/svg/custom/recursive-use.svg:10 > +<text>Don't crash!</text> PASS would be better text here.
(In reply to comment #5) > (From update of attachment 77531 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77531&action=review > > > LayoutTests/svg/custom/recursive-use.svg:10 > > +<text>Don't crash!</text> > > PASS would be better text here. Hm. Indeed. Maybe "PASS. Test didn't crash." Leo can you upload a new patch please, since you do not have commit rights yet?
Created attachment 77862 [details] Revised patch version 2
Comment on attachment 77862 [details] Revised patch version 2 LGTM. r=me
Comment on attachment 77862 [details] Revised patch version 2 Clearing flags on attachment: 77862 Committed r74960: <http://trac.webkit.org/changeset/74960>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/74960 might have broken GTK Linux 64-bit Debug The following tests are not passing: svg/custom/use-nested-children.svg
(In reply to comment #11) > http://trac.webkit.org/changeset/74960 might have broken GTK Linux 64-bit Debug > The following tests are not passing: > svg/custom/use-nested-children.svg This case passes. Reverified based on 74962.
(In reply to comment #12) > (In reply to comment #11) > > http://trac.webkit.org/changeset/74960 might have broken GTK Linux 64-bit Debug > > The following tests are not passing: > > svg/custom/use-nested-children.svg > > This case passes. Reverified based on 74962. It seems hitting an assertion. Testing with debug build.(In reply to comment #12) > (In reply to comment #11) > > http://trac.webkit.org/changeset/74960 might have broken GTK Linux 64-bit Debug > > The following tests are not passing: > > svg/custom/use-nested-children.svg > > This case passes. Reverified based on 74962. It seems hitting an assertion. Testing with debug build.
Created attachment 77978 [details] Revised patch version 3
Comment on attachment 77978 [details] Revised patch version 3 View in context: https://bugs.webkit.org/attachment.cgi?id=77978&action=review just a snippet > WebCore/svg/SVGUseElement.cpp:726 > - handleDeepUseReferencing(static_cast<SVGUseElement*>(target), targetInstance, foundProblem); > + if (targetHasUseTag && newTarget) { > + RefPtr<SVGElementInstance> newInstance = SVGElementInstance::create(this, newTarget); > + SVGElementInstance* newInstancePtr = newInstance.get(); > + targetInstance->appendChild(newInstance.release()); > + buildInstanceTree(newTarget, newInstancePtr, foundProblem); > + } should be if (!targetHasUseTag || !newTarget) return;
reopen bug
Created attachment 77983 [details] Revised patch version 4
Comment on attachment 77983 [details] Revised patch version 4 r=me
Comment on attachment 77983 [details] Revised patch version 4 Clearing flags on attachment: 77983 Committed r75059: <http://trac.webkit.org/changeset/75059>
http://trac.webkit.org/changeset/75059 might have broken Leopard Intel Debug (Tests)