From 0af968274de50739772bf68a30f23e7046e1acd1 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Thu, 23 Jul 2026 11:15:50 +0200 Subject: [PATCH] QQuickSvgParser: respect end iterator when skipping whitespace The code contained a number of while loops to eat all whitespace or parse all digits without respecting the end-sentinel. An input string ending with whitespace or digit results in a possibly unterminated out-of-bounds read. To fix this in parsePathDataFast, remove the special handling of whitespace in this code, and instead just continue the outer loop if the current character is whitespace. Pass the end sentinel to the parseNumbersArray and toDouble helpers so that we can consistently test whether incrementing the str point has reached the end. Add test coverage, both to the QML svgpath test case, and to the QQuickPath test using the private parsePathDataFast API directly. The input data is the same in both cases. Note that running these tests without the fix might only fail in an ASAN-enabled build. Backport to Qt 6.8 requires exporting the parsePathDataFast function for auto-tests and making the new test conditional to internal builds. Pick-to: 6.5 5.15 Fixes: QTBUG-148526 Change-Id: I3623e97510926b831daaa8b85e1fd5496faf27a1 Reviewed-by: Dimitrios Apostolou Reviewed-by: Hatem ElKharashy (cherry picked from commit 9ead7460ef95321c837b161b8f7951546711d03e) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 1854ebceebae5af54d9e32509d61e838c8643585) (cherry picked from commit 0aba8c7674dbbbaaa142775247249e297a339c85) Reviewed-by: Volker Hilsheimer --- diff --git a/src/quick/util/qquicksvgparser.cpp b/src/quick/util/qquicksvgparser.cpp index f7ed97f..5c4f0e3 100644 --- a/src/quick/util/qquicksvgparser.cpp +++ b/src/quick/util/qquicksvgparser.cpp @@ -19,7 +19,7 @@ return ((ch >> 4) == 3) && (magic >> (ch & 15)); } -static qreal toDouble(const QChar *&str) +static qreal toDouble(const QChar *&str, const QChar *end) { const int maxLen = 255;//technically doubles can go til 308+ but whatever char temp[maxLen+1]; @@ -31,28 +31,28 @@ } else if (*str == QLatin1Char('+')) { ++str; } - while (isDigit(str->unicode()) && pos < maxLen) { + while (str != end && isDigit(str->unicode()) && pos < maxLen) { temp[pos++] = str->toLatin1(); ++str; } - if (*str == QLatin1Char('.') && pos < maxLen) { + if (str != end && *str == QLatin1Char('.') && pos < maxLen) { temp[pos++] = '.'; ++str; } - while (isDigit(str->unicode()) && pos < maxLen) { + while (str != end && isDigit(str->unicode()) && pos < maxLen) { temp[pos++] = str->toLatin1(); ++str; } bool exponent = false; - if ((*str == QLatin1Char('e') || *str == QLatin1Char('E')) && pos < maxLen) { + if (str != end && (*str == QLatin1Char('e') || *str == QLatin1Char('E')) && pos < maxLen) { exponent = true; temp[pos++] = 'e'; ++str; - if ((*str == QLatin1Char('-') || *str == QLatin1Char('+')) && pos < maxLen) { + if (str != end && (*str == QLatin1Char('-') || *str == QLatin1Char('+')) && pos < maxLen) { temp[pos++] = str->toLatin1(); ++str; } - while (isDigit(str->unicode()) && pos < maxLen) { + while (str != end && isDigit(str->unicode()) && pos < maxLen) { temp[pos++] = str->toLatin1(); ++str; } @@ -96,24 +96,28 @@ return val; } -static inline void parseNumbersArray(const QChar *&str, QVarLengthArray &points) +static inline void parseNumbersArray(const QChar *&str, const QChar *end, QVarLengthArray &points) { - while (str->isSpace()) - ++str; - while (isDigit(str->unicode()) || - *str == QLatin1Char('-') || *str == QLatin1Char('+') || - *str == QLatin1Char('.')) { - - points.append(toDouble(str)); - - while (str->isSpace()) + auto eatWhitespace = [&]{ + while (str != end && str->isSpace()) ++str; + return str != end; + }; + if (!eatWhitespace()) + return; + while (str != end && (isDigit(str->unicode()) || + *str == QLatin1Char('-') || *str == QLatin1Char('+') || + *str == QLatin1Char('.'))) { + + points.append(toDouble(str, end)); + + if (!eatWhitespace()) + return; if (*str == QLatin1Char(',')) ++str; - //eat the rest of space - while (str->isSpace()) - ++str; + if (!eatWhitespace()) + return; } } @@ -253,12 +257,12 @@ const QChar *end = str + dataStr.size(); while (str != end) { - while (str->isSpace()) - ++str; QChar pathElem = *str; ++str; + if (pathElem.isSpace()) + continue; QVarLengthArray arg; - parseNumbersArray(str, arg); + parseNumbersArray(str, end, arg); if (pathElem == QLatin1Char('z') || pathElem == QLatin1Char('Z')) arg.append(0);//dummy const qreal *num = arg.constData(); diff --git a/src/quick/util/qquicksvgparser_p.h b/src/quick/util/qquicksvgparser_p.h index 482d0d4..a38fa26 100644 --- a/src/quick/util/qquicksvgparser_p.h +++ b/src/quick/util/qquicksvgparser_p.h @@ -24,7 +24,7 @@ namespace QQuickSvgParser { - bool parsePathDataFast(const QString &dataStr, QPainterPath &path); + Q_QUICK_AUTOTEST_EXPORT bool parsePathDataFast(const QString &dataStr, QPainterPath &path); Q_QUICK_EXPORT void pathArc(QPainterPath &path, qreal rx, qreal ry, qreal x_axis_rotation, int large_arc_flag, int sweep_flag, qreal x, qreal y, qreal curx, qreal cury); diff --git a/tests/auto/quick/qquickcanvasitem/data/tst_svgpath.qml b/tests/auto/quick/qquickcanvasitem/data/tst_svgpath.qml index 5966114..ec3a3b9 100644 --- a/tests/auto/quick/qquickcanvasitem/data/tst_svgpath.qml +++ b/tests/auto/quick/qquickcanvasitem/data/tst_svgpath.qml @@ -56,4 +56,47 @@ } } } + + // The test only verifies that assigning malformed strings (esp those ending + // with whitespace) to ctx.path does not crash the parser. + function test_svgpath_malformed_data() { + return [ + { tag: "empty", path: "" }, + { tag: "single space", path: " " }, + { tag: "multiple spaces", path: " " }, + { tag: "abs and newlines", path: "\t\n "}, + { tag: "command then trailing whitespace", path: "M0 0 "}, + { tag: "trailing whitespace after args", path: "M0 0 L10 10 \n "}, + { tag: "whitespace after command to end", path: "L \t"}, + { tag: "whitespace between command and numbers", path: "M 10 20 "}, + { tag: "trailing comma to end", path: "M0 0,"}, + { tag: "comma then whitespace to end", path: "M0 0, \n"}, + { tag: "comma between numbers then end", path: "L10,10, "}, + { tag: "integer to end", path: "M0 0 L10 20"}, + { tag: "decimal to end", path: "M0 0 L1.5 2.5"}, + { tag: "trailing dot to end", path: "M0 0 L1. 2."}, + { tag: "exponent to end", path: "M0 0 L1e2 3e2"}, + { tag: "signed exponent to end", path: "M0 0 L1e-2 3e-2"}, + { tag: "bare exponent letter to end", path: "M0 0 L1e"}, + { tag: "sign only token to end", path: "M0 0 L-"}, + { tag: "dot only token to end", path: "M0 0 L."} + ] + } + + function test_svgpath_malformed(data) { + var canvas = Qt.createQmlObject(` + import QtQuick + Canvas { + height: 100 + width:100 + renderTarget:Canvas.Image + } + `, testCase, "testCanvas"); + tryVerify(function() { return canvas.available; }); + var ctx = canvas.getContext('2d'); + ctx.beginPath(); + ctx.path = data.path; + ctx.fill(); + verify(true); // reached here without crashing + } } diff --git a/tests/auto/quick/qquickpath/tst_qquickpath.cpp b/tests/auto/quick/qquickpath/tst_qquickpath.cpp index 7ed9ad3..856f36a 100644 --- a/tests/auto/quick/qquickpath/tst_qquickpath.cpp +++ b/tests/auto/quick/qquickpath/tst_qquickpath.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include @@ -21,6 +22,8 @@ void catmullRomCurve(); void closedCatmullRomCurve(); void svg(); + void svgMalformed_data(); + void svgMalformed(); void line(); void rectangle_data(); void rectangle(); @@ -277,6 +280,47 @@ svg(QSizeF(5,3)); } +void tst_QuickPath::svgMalformed_data() +{ + QTest::addColumn("svgPath"); + +#ifdef QT_BUILD_INTERNAL + // Malformed path strings must not crash the parser. + QTest::newRow("empty") << QString(); + QTest::newRow("single space") << QStringLiteral(" "); + QTest::newRow("multiple spaces") << QStringLiteral(" "); + QTest::newRow("tabs and newlines") << QStringLiteral("\t\n "); + QTest::newRow("command then trailing whitespace") << QStringLiteral("M0 0 "); + QTest::newRow("trailing whitespace after args") << QStringLiteral("M0 0 L10 10 \n "); + QTest::newRow("whitespace after command to end") << QStringLiteral("L \t"); + QTest::newRow("whitespace between command and numbers") << QStringLiteral("M 10 20 "); + QTest::newRow("trailing comma to end") << QStringLiteral("M0 0,"); + QTest::newRow("comma then whitespace to end") << QStringLiteral("M0 0, \n"); + QTest::newRow("comma between numbers then end") << QStringLiteral("L10,10, "); + QTest::newRow("integer to end") << QStringLiteral("M0 0 L10 20"); + QTest::newRow("decimal to end") << QStringLiteral("M0 0 L1.5 2.5"); + QTest::newRow("trailing dot to end") << QStringLiteral("M0 0 L1. 2."); + QTest::newRow("exponent to end") << QStringLiteral("M0 0 L1e2 3e2"); + QTest::newRow("signed exponent to end") << QStringLiteral("M0 0 L1e-2 3e-2"); + QTest::newRow("bare exponent letter to end") << QStringLiteral("M0 0 L1e"); + QTest::newRow("sign only token to end") << QStringLiteral("M0 0 L-"); + QTest::newRow("dot only token to end") << QStringLiteral("M0 0 L."); +#else + QSKIP("This test relies on private APIs that are only exported in developer-builds"); +#endif +} + +void tst_QuickPath::svgMalformed() +{ +#ifdef QT_BUILD_INTERNAL + QFETCH(QString, svgPath); + QPainterPath path; + QQuickSvgParser::parsePathDataFast(svgPath, path); +#else + QSKIP("This test relies on private APIs that are only exported in developer-builds"); +#endif +} + void tst_QuickPath::line(QSizeF scale) { QQmlEngine engine;