diff --git a/CVE-2025-10728-qtsvg-6.8.diff b/CVE-2025-10728-qtsvg-6.8.diff new file mode 100644 index 0000000000000000000000000000000000000000..1ece47bb569d186162b53e32324dda5c64ebfe14 --- /dev/null +++ b/CVE-2025-10728-qtsvg-6.8.diff @@ -0,0 +1,112 @@ +From ee5c5744242435d10360a23f0c1798a45f38a518 Mon Sep 17 00:00:00 2001 +From: Robert Löhning +Date: Wed, 18 Jun 2025 16:02:32 +0200 +Subject: [PATCH] Replace check for endless recursion when loading + +The old check parsed the tree of SvgNodes again and again which lead to +quadratic complexity. Instead, set and check a bool where the recursion +may actually happen which is faster and only has linear complexity. + +Partially reverts 0332df304f013ded362537c1f61556098b875352 + +I chose to have the check in QSvgPattern::renderPattern() because: + +- It not only appears in the recursive backtrace of the stack-overflow + which was fixed using the qudratic check, but also in the backtrace + of another, still unfixed stack overflow. That way, both can be fixed + by the same patch. Credit to OSS-Fuzz for finding them. +- The function already had some error checking and returns a default + value when it cannot render the content. In the same way, I can return + a QImage of the right size but without any content when the endless + recursion is about to happen. + +[ChangeLog] Speed up loading by replacing check for cyclic elements +[ChangeLog] Fix stack overflow when an element references its child +element using url() + +Fixes: QTBUG-137553 +Change-Id: If011c15fde50dcefeb653d1d5995ff1347e7b5ac +Reviewed-by: Hatem ElKharashy +(cherry picked from commit 9e5bed9584ab65d56cd5fbac0471e06e37a54412) +Reviewed-by: Qt Cherry-pick Bot +(cherry picked from commit 90a5331640bb760b0114a7ea4e08b9e42b03e082) +(cherry picked from commit ea44b50c6e61104cadd6b7c8ede92a4108634232) +--- + +diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp +index e227e00..b3ecbbc 100644 +--- a/src/svg/qsvghandler.cpp ++++ b/src/svg/qsvghandler.cpp +@@ -4662,8 +4662,7 @@ void QSvgHandler::parse() + // this point is to do what everyone else seems to do and + // ignore the reported namespaceUri completely. + if (remainingUnfinishedElements +- && startElement(xml->name().toString(), xml->attributes()) +- && !detectCyclesAndWarn(m_doc)) { ++ && startElement(xml->name().toString(), xml->attributes())) { + --remainingUnfinishedElements; + } else { + delete m_doc; +diff --git a/src/svg/qsvgstructure.cpp b/src/svg/qsvgstructure.cpp +index 45824da..08f4110 100644 +--- a/src/svg/qsvgstructure.cpp ++++ b/src/svg/qsvgstructure.cpp +@@ -802,6 +802,7 @@ QSvgPattern::QSvgPattern(QSvgNode *parent, QSvgRectF bounds, QRectF viewBox, + m_rect(bounds), + m_viewBox(viewBox), + m_contentUnits(contentUnits), ++ m_isRendering(false), + m_transform(transform) + + { +@@ -887,6 +888,13 @@ QImage QSvgPattern::renderPattern(QSize size, qreal contentScaleX, qreal content + } + pattern.fill(Qt::transparent); + ++ if (m_isRendering) { ++ qCWarning(lcSvgDraw) << "The pattern is trying to render itself recursively. " ++ "Returning a transparent QImage of the right size."; ++ return pattern; ++ } ++ QScopedValueRollback guard(m_isRendering, true); ++ + // Draw the pattern using our QPainter. + QPainter patternPainter(&pattern); + QSvgExtraStates patternStates; +diff --git a/src/svg/qsvgstructure_p.h b/src/svg/qsvgstructure_p.h +index 63a8396..f95508f 100644 +--- a/src/svg/qsvgstructure_p.h ++++ b/src/svg/qsvgstructure_p.h +@@ -241,6 +241,7 @@ private: + QSvgRectF m_rect; + QRectF m_viewBox; + QtSvg::UnitTypes m_contentUnits; ++ mutable bool m_isRendering; + QTransform m_transform; + }; + +diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +index 6cb1c55..2039de0 100644 +--- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp ++++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +@@ -67,6 +67,7 @@ private slots: + void oss_fuzz_24738(); + void oss_fuzz_61586(); + void oss_fuzz_42532991(); ++ void oss_fuzz_390467765(); + void oss_fuzz_399769595(); + void imageRendering(); + void illegalAnimateTransform_data(); +@@ -1740,6 +1741,12 @@ void tst_QSvgRenderer::oss_fuzz_42532991() + QSvgRenderer().load(QByteArray("")); + } + ++void tst_QSvgRenderer::oss_fuzz_390467765() ++{ ++ // resulted in stack overflow ++ QSvgRenderer().load(QByteArray("")); ++} ++ + void tst_QSvgRenderer::oss_fuzz_399769595() + { + // resulted in null pointer deref diff --git a/CVE-2025-10729-qtsvg-6.8.diff b/CVE-2025-10729-qtsvg-6.8.diff new file mode 100644 index 0000000000000000000000000000000000000000..eaee3ccaecfad44fab52f4147de2a6947829aea2 --- /dev/null +++ b/CVE-2025-10729-qtsvg-6.8.diff @@ -0,0 +1,147 @@ +From 70959f3a54e940028ce4f9b49e033e3e67db79a8 Mon Sep 17 00:00:00 2001 +From: Robert Löhning +Date: Fri, 12 Sep 2025 21:22:58 +0200 +Subject: [PATCH] Don't create group nodes which will be deleted anyway + +The old code first created the nodes, then checked whether their parent +element has the right type and deleted them if not. This was wasted +effort and could also lead to dangling pointers. + +Instead, first check the parent's type and only create the node if that +matches. + +Task-number: QTBUG-139961 +Change-Id: Ifa870efbd5f336b34b81aa09b6fe79fb7fc826b9 +Reviewed-by: Hatem ElKharashy +(cherry picked from commit 7e8898903265d931df0aa54b3913f2c49d4d7bf2) +Reviewed-by: Jani Heikkinen +(cherry picked from commit 16a8eb2c88ec1125c897da35b524a18d619eb261) +(cherry picked from commit 6a6273126770006232e805cf1631f93d4919b788) +Reviewed-by: Qt Cherry-pick Bot +--- + +diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp +index 4553c46..0c5d74d 100644 +--- a/src/svg/qsvghandler.cpp ++++ b/src/svg/qsvghandler.cpp +@@ -4733,46 +4733,46 @@ + + if (FactoryMethod method = findGroupFactory(localName, options())) { + //group +- node = method(m_doc ? m_nodes.top() : 0, attributes, this); +- +- if (node) { +- if (!m_doc) { ++ if (!m_doc) { ++ node = method(nullptr, attributes, this); ++ if (node) { + Q_ASSERT(node->type() == QSvgNode::Doc); + m_doc = static_cast(node); +- } else { +- switch (m_nodes.top()->type()) { +- case QSvgNode::Doc: +- case QSvgNode::Group: +- case QSvgNode::Defs: +- case QSvgNode::Switch: +- case QSvgNode::Mask: +- case QSvgNode::Symbol: +- case QSvgNode::Marker: +- case QSvgNode::Pattern: +- { ++ } ++ } else { ++ switch (m_nodes.top()->type()) { ++ case QSvgNode::Doc: ++ case QSvgNode::Group: ++ case QSvgNode::Defs: ++ case QSvgNode::Switch: ++ case QSvgNode::Mask: ++ case QSvgNode::Symbol: ++ case QSvgNode::Marker: ++ case QSvgNode::Pattern: ++ { ++ node = method(m_nodes.top(), attributes, this); ++ if (node) { + QSvgStructureNode *group = + static_cast(m_nodes.top()); + group->addChild(node, someId(attributes)); + } +- break; +- default: +- const QByteArray msg = QByteArrayLiteral("Could not add child element to parent element because the types are incorrect."); +- qCWarning(lcSvgHandler, "%s", prefixMessage(msg, xml).constData()); +- delete node; +- node = 0; +- break; +- } + } ++ break; ++ default: ++ const QByteArray msg = QByteArrayLiteral("Could not add child element to parent element because the types are incorrect."); ++ qCWarning(lcSvgHandler, "%s", prefixMessage(msg, xml).constData()); ++ break; ++ } ++ } + +- if (node) { +- parseCoreNode(node, attributes); ++ if (node) { ++ parseCoreNode(node, attributes); + #ifndef QT_NO_CSSPARSER +- cssStyleLookup(node, this, m_selector); ++ cssStyleLookup(node, this, m_selector); + #endif +- parseStyle(node, attributes, this); +- if (node->type() == QSvgNode::Filter) +- m_toBeResolved.append(node); +- } ++ parseStyle(node, attributes, this); ++ if (node->type() == QSvgNode::Filter) ++ m_toBeResolved.append(node); + } + } else if (FactoryMethod method = findGraphicsFactory(localName, options())) { + //rendering element +diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +index dd677ef..f32362c 100644 +--- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp ++++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +@@ -82,6 +82,7 @@ + void testSymbol(); + void testMarker(); + void testPatternElement(); ++ void testMisplacedElement(); + void testCycles(); + void testFeFlood(); + void testFeOffset(); +@@ -2091,6 +2092,31 @@ + QCOMPARE(refImage, image); + } + ++void tst_QSvgRenderer::testMisplacedElement() ++{ ++ // This input caused a QSvgPattern node to be created with a QSvgPatternStyle referencing to it. ++ // The code then detected that the element is misplaced in the element and ++ // deleted it. That left behind the QSvgPatternStyle pointing to the deleted QSvgPattern. That ++ // was reported when running the test with ASAN or UBSAN. ++ QByteArray svg(R"( ++ ++ ++ )"); ++ ++ QImage image(20, 20, QImage::Format_ARGB32_Premultiplied); ++ image.fill(Qt::green); ++ QImage refImage = image.copy(); ++ ++ QTest::ignoreMessage(QtWarningMsg, "