From 0aba8c7674dbbbaaa142775247249e297a339c85 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. Pick-to: 6.8 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) --- diff --git a/src/quick/util/qquicksvgparser.cpp b/src/quick/util/qquicksvgparser.cpp index 59a976b..84efe06 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/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 c867243..4ac0e83 100644 --- a/tests/auto/quick/qquickpath/tst_qquickpath.cpp +++ b/tests/auto/quick/qquickpath/tst_qquickpath.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include @@ -22,6 +23,8 @@ void catmullRomCurve(); void closedCatmullRomCurve(); void svg(); + void svgMalformed_data(); + void svgMalformed(); void line(); void rectangle_data(); void rectangle(); @@ -282,6 +285,39 @@ svg(QSizeF(5,3)); } +void tst_QuickPath::svgMalformed_data() +{ + QTest::addColumn("svgPath"); + + // 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."); +} + +void tst_QuickPath::svgMalformed() +{ + QFETCH(QString, svgPath); + QPainterPath path; + QQuickSvgParser::parsePathDataFast(svgPath, path); +} + void tst_QuickPath::line(QSizeF scale) { QQmlEngine engine;