From 36cfd9efb9b22b891adee9c48d30202289cfa620 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Mon, 25 Oct 2021 14:17:55 +0200 Subject: 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 Reviewed-by: Robert Löhning --- src/svg/qsvghandler.cpp | 59 +++++++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp index db29211..dd869ff 100644 --- a/src/svg/qsvghandler.cpp +++ b/src/svg/qsvghandler.cpp @@ -1595,6 +1595,7 @@ static void pathArc(QPainterPath &path, static bool parsePathDataFast(QStringView 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; @@ -1602,7 +1603,8 @@ static bool parsePathDataFast(QStringView 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; @@ -1619,14 +1621,13 @@ static bool parsePathDataFast(QStringView 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; @@ -1643,8 +1644,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) break; case 'M': { if (count < 2) { - num++; - count--; + ok = false; break; } x = x0 = num[0]; @@ -1670,8 +1670,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) break; case 'l': { if (count < 2) { - num++; - count--; + ok = false; break; } x = num[0] + offsetX; @@ -1684,8 +1683,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) break; case 'L': { if (count < 2) { - num++; - count--; + ok = false; break; } x = num[0]; @@ -1725,8 +1723,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) break; case 'c': { if (count < 6) { - num += count; - count = 0; + ok = false; break; } QPointF c1(num[0] + offsetX, num[1] + offsetY); @@ -1742,8 +1739,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 'C': { if (count < 6) { - num += count; - count = 0; + ok = false; break; } QPointF c1(num[0], num[1]); @@ -1759,8 +1755,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 's': { if (count < 4) { - num += count; - count = 0; + ok = false; break; } QPointF c1; @@ -1781,8 +1776,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 'S': { if (count < 4) { - num += count; - count = 0; + ok = false; break; } QPointF c1; @@ -1803,8 +1797,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 'q': { if (count < 4) { - num += count; - count = 0; + ok = false; break; } QPointF c(num[0] + offsetX, num[1] + offsetY); @@ -1819,8 +1812,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 'Q': { if (count < 4) { - num += count; - count = 0; + ok = false; break; } QPointF c(num[0], num[1]); @@ -1835,8 +1827,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 't': { if (count < 2) { - num += count; - count = 0; + ok = false; break; } QPointF e(num[0] + offsetX, num[1] + offsetY); @@ -1856,8 +1847,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 'T': { if (count < 2) { - num += count; - count = 0; + ok = false; break; } QPointF e(num[0], num[1]); @@ -1877,8 +1867,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) } case 'a': { if (count < 7) { - num += count; - count = 0; + ok = false; break; } qreal rx = (*num++); @@ -1900,8 +1889,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path) break; case 'A': { if (count < 7) { - num += count; - count = 0; + ok = false; break; } qreal rx = (*num++); @@ -1922,12 +1910,15 @@ static bool parsePathDataFast(QStringView 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, @@ -2985,8 +2976,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; -- cgit v1.2.1