From 18a2adad907523ea31251ff0b62e3321241a40fa Mon Sep 17 00:00:00 2001
From: Eirik Aavitsland <eirik.aavitsland@qt.io>
Date: Tue, 14 Nov 2017 14:47:08 +0100
Subject: Fix crash on recursive self-referral in <use> element

Referring to an ancestor in a <use> element would lead to endless
recursion. Add checks to avoid recursion, and also emit a warning
while parsing.

Task-number: QTBUG-64425
Change-Id: I9ee1b9bfef13796cc3f387ff8579c6b13bc4ae9a
Reviewed-by: Andy Shaw <andy.shaw@qt.io>
---
 src/svg/qsvggraphics.cpp                     |  5 ++++-
 src/svg/qsvghandler.cpp                      |  2 ++
 src/svg/qsvgnode.cpp                         | 11 +++++++++++
 src/svg/qsvgnode_p.h                         |  1 +
 tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp | 15 ++++++++++++++-
 5 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/svg/qsvggraphics.cpp b/src/svg/qsvggraphics.cpp
index b1047eb..5b273af 100644
--- a/src/svg/qsvggraphics.cpp
+++ b/src/svg/qsvggraphics.cpp
@@ -465,6 +465,9 @@ QSvgUse::QSvgUse(const QPointF &start, QSvgNode *parent, QSvgNode *node)
 
 void QSvgUse::draw(QPainter *p, QSvgExtraStates &states)
 {
+    if (Q_UNLIKELY(!m_link || isDescendantOf(m_link)))
+        return;
+
     applyStyle(p, states);
 
     if (!m_start.isNull()) {
@@ -553,7 +556,7 @@ QSvgNode::Type QSvgVideo::type() const
 QRectF QSvgUse::bounds(QPainter *p, QSvgExtraStates &states) const
 {
     QRectF bounds;
-    if (m_link) {
+    if (Q_LIKELY(m_link && !isDescendantOf(m_link))) {
         p->translate(m_start);
         bounds = m_link->transformedBounds(p, states);
         p->translate(-m_start);
diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp
index 7841fa3..022afb6 100644
--- a/src/svg/qsvghandler.cpp
+++ b/src/svg/qsvghandler.cpp
@@ -3338,6 +3338,8 @@ static QSvgNode *createUseNode(QSvgNode *parent,
     if (group) {
         QSvgNode *link = group->scopeNode(linkId);
         if (link) {
+            if (parent->isDescendantOf(link))
+                qCWarning(lcSvgHandler, "link #%s is recursive!", qPrintable(linkId));
             QPointF pt;
             if (!xStr.isNull() || !yStr.isNull()) {
                 QSvgHandler::LengthType type;
diff --git a/src/svg/qsvgnode.cpp b/src/svg/qsvgnode.cpp
index e504522..14aaea4 100644
--- a/src/svg/qsvgnode.cpp
+++ b/src/svg/qsvgnode.cpp
@@ -57,6 +57,17 @@ QSvgNode::~QSvgNode()
 
 }
 
+bool QSvgNode::isDescendantOf(const QSvgNode *parent) const
+{
+    const QSvgNode *n = this;
+    while (n) {
+        if (n == parent)
+            return true;
+        n = n->m_parent;
+    }
+    return false;
+}
+
 void QSvgNode::appendStyleProperty(QSvgStyleProperty *prop, const QString &id)
 {
     //qDebug()<<"appending "<<prop->type()<< " ("<< id <<") "<<"to "<<this<<this->type();
diff --git a/src/svg/qsvgnode_p.h b/src/svg/qsvgnode_p.h
index f9f6202..f2502e4 100644
--- a/src/svg/qsvgnode_p.h
+++ b/src/svg/qsvgnode_p.h
@@ -113,6 +113,7 @@ public:
     virtual void draw(QPainter *p, QSvgExtraStates &states) =0;
 
     QSvgNode *parent() const;
+    bool isDescendantOf(const QSvgNode *parent) const;
 
     void appendStyleProperty(QSvgStyleProperty *prop, const QString &id);
     void applyStyle(QPainter *p, QSvgExtraStates &states) const;
diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp
index 6ac865a..fd1b350 100644
--- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp
+++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp
@@ -1308,6 +1308,17 @@ void tst_QSvgRenderer::testUseElement()
         " <g fill = \"red\" fill-opacity =\"0.5\">"
         "  <use xlink:href =\"#usedG\" />"
         " </g>"
+        "</svg>",
+        // Self referral, should be ignored
+        "<svg><g id=\"0\"><use xlink:href=\"#0\" /></g></svg>",
+        "<svg width=\"200\" height=\"200\">"
+        "  <rect width=\"100\" height=\"50\"/>"
+        "</svg>",
+        "<svg width=\"200\" height=\"200\">"
+        "  <g id=\"0\"><use xlink:href=\"#0\" /><rect width=\"100\" height=\"50\"/></g>"
+        "</svg>",
+        "<svg width=\"200\" height=\"200\">"
+        "  <g id=\"0\"><g><use xlink:href=\"#0\" /><rect width=\"100\" height=\"50\"/></g></g>"
         "</svg>"
     };
 
@@ -1334,8 +1345,10 @@ void tst_QSvgRenderer::testUseElement()
                 // For this reason an exact comparison will fail.
                 QCOMPARE(images[4], images[i]);
             }
-        } else if (i > 7) {
+        } else if (i > 7 && i < 10) {
             QCOMPARE(images[8], images[i]);
+        } else if (i > 11) {
+            QCOMPARE(images[11], images[i]);
         }
     }
 }
-- 
cgit v1.2.1