From 5b9285c34731e67f9f1d61ec804740991f2a0380 Mon Sep 17 00:00:00 2001

From: Eirik Aavitsland <eirik.aavitsland@qt.io>

Date: Mon, 25 Oct 2021 14:17:55 +0200

Subject: [PATCH] Do stricter error checking when parsing path nodes

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

The SVG spec mandates that path parsing should terminate on the first

error encountered, and an error be reported. To improve the handling

of corrupt files, implement such error handling, and also limit the

number of QPainterPath elements to a reasonable range.

Fixes: QTBUG-96044

Pick-to: 6.2 5.15 5.12

Change-Id: Ic5e65d6b658516d6f1317c72de365c8c7ad81891

Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

Reviewed-by: Robert Löhning <robert.loehning@qt.io>

(cherry picked from commit 36cfd9efb9b22b891adee9c48d30202289cfa620)

---

 src/svg/qsvghandler.cpp | 59 +++++++++++++++++------------------------

 1 file changed, 25 insertions(+), 34 deletions(-)

diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp

index b542089..2ea80ed 100644

--- a/src/svg/qsvghandler.cpp

+++ b/src/svg/qsvghandler.cpp

@@ -1627,6 +1627,7 @@ static void pathArc(QPainterPath &path,

 static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

 {

+    const int maxElementCount = 0x7fff; // Assume file corruption if more path elements than this

     qreal x0 = 0, y0 = 0;              // starting point

     qreal x = 0, y = 0;                // current point

     char lastMode = 0;

@@ -1634,7 +1635,8 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

     const QChar *str = dataStr.constData();

     const QChar *end = str + dataStr.size();

-    while (str != end) {

+    bool ok = true;

+    while (ok && str != end) {

         while (str->isSpace() && (str + 1) != end)

             ++str;

         QChar pathElem = *str;

@@ -1651,14 +1653,13 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             arg.append(0);//dummy

         const qreal *num = arg.constData();

         int count = arg.count();

-        while (count > 0) {

+        while (ok && count > 0) {

             qreal offsetX = x;        // correction offsets

             qreal offsetY = y;        // for relative commands

             switch (pathElem.unicode()) {

             case 'm': {

                 if (count < 2) {

-                    num++;

-                    count--;

+                    ok = false;

                     break;

                 }

                 x = x0 = num[0] + offsetX;

@@ -1675,8 +1676,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

                 break;

             case 'M': {

                 if (count < 2) {

-                    num++;

-                    count--;

+                    ok = false;

                     break;

                 }

                 x = x0 = num[0];

@@ -1702,8 +1702,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

                 break;

             case 'l': {

                 if (count < 2) {

-                    num++;

-                    count--;

+                    ok = false;

                     break;

                 }

                 x = num[0] + offsetX;

@@ -1716,8 +1715,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

                 break;

             case 'L': {

                 if (count < 2) {

-                    num++;

-                    count--;

+                    ok = false;

                     break;

                 }

                 x = num[0];

@@ -1757,8 +1755,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

                 break;

             case 'c': {

                 if (count < 6) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF c1(num[0] + offsetX, num[1] + offsetY);

@@ -1774,8 +1771,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 'C': {

                 if (count < 6) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF c1(num[0], num[1]);

@@ -1791,8 +1787,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 's': {

                 if (count < 4) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF c1;

@@ -1813,8 +1808,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 'S': {

                 if (count < 4) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF c1;

@@ -1835,8 +1829,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 'q': {

                 if (count < 4) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF c(num[0] + offsetX, num[1] + offsetY);

@@ -1851,8 +1844,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 'Q': {

                 if (count < 4) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF c(num[0], num[1]);

@@ -1867,8 +1859,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 't': {

                 if (count < 2) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF e(num[0] + offsetX, num[1] + offsetY);

@@ -1888,8 +1879,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 'T': {

                 if (count < 2) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 QPointF e(num[0], num[1]);

@@ -1909,8 +1899,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

             case 'a': {

                 if (count < 7) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 qreal rx = (*num++);

@@ -1932,8 +1921,7 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

                 break;

             case 'A': {

                 if (count < 7) {

-                    num += count;

-                    count = 0;

+                    ok = false;

                     break;

                 }

                 qreal rx = (*num++);

@@ -1954,12 +1942,15 @@ static bool parsePathDataFast(const QStringRef &dataStr, QPainterPath &path)

             }

                 break;

             default:

-                return false;

+                ok = false;

+                break;

             }

             lastMode = pathElem.toLatin1();

+            if (path.elementCount() > maxElementCount)

+                ok = false;

         }

     }

-    return true;

+    return ok;

 }

 static bool parseStyle(QSvgNode *node,

@@ -2997,8 +2988,8 @@ static QSvgNode *createPathNode(QSvgNode *parent,

     QPainterPath qpath;

     qpath.setFillRule(Qt::WindingFill);

-    //XXX do error handling

-    parsePathDataFast(data, qpath);

+    if (!parsePathDataFast(data, qpath))

+        qCWarning(lcSvgHandler, "Invalid path data; path truncated.");

     QSvgNode *path = new QSvgPath(parent, qpath);

     return path;

--

GitLab