|
| 1 | +From b4112e9c8ff75ca03de65236058af41598308ca3 Mon Sep 17 00:00:00 2001 |
| 2 | +From: AllSpark < [email protected]> |
| 3 | +Date: Tue, 7 Oct 2025 10:08:13 +0000 |
| 4 | +Subject: [PATCH] Don't create group nodes which will be deleted anyway; add |
| 5 | + test for misplaced element. Task-number: QTBUG-139961 |
| 6 | + |
| 7 | +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. |
| 8 | + |
| 9 | +Instead, first check the parent's type and only create the node if that matches. |
| 10 | + |
| 11 | +Reviewed-by: Hatem ElKharashy < [email protected]> |
| 12 | +Reviewed-by: Jani Heikkinen < [email protected]> |
| 13 | +Signed-off-by: Azure Linux Security Servicing Account < [email protected]> |
| 14 | +Upstream-reference: AI Backport of https://code.qt.io/cgit/qt/qtsvg.git/patch/?id=6a6273126770006232e805cf1631f93d4919b788 |
| 15 | +--- |
| 16 | + src/svg/qsvghandler.cpp | 20 ++++++++------ |
| 17 | + tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp | 29 ++++++++++++++++++++ |
| 18 | + 2 files changed, 40 insertions(+), 9 deletions(-) |
| 19 | + |
| 20 | +diff --git a/src/svg/qsvghandler.cpp b/src/svg/qsvghandler.cpp |
| 21 | +index 0b3341a..ed6258d 100644 |
| 22 | +--- a/src/svg/qsvghandler.cpp |
| 23 | ++++ b/src/svg/qsvghandler.cpp |
| 24 | +@@ -3730,11 +3730,12 @@ bool QSvgHandler::startElement(const QString &localName, |
| 25 | + |
| 26 | + if (FactoryMethod method = findGroupFactory(localName)) { |
| 27 | + //group |
| 28 | +- node = method(m_doc ? m_nodes.top() : 0, attributes, this); |
| 29 | +- Q_ASSERT(node); |
| 30 | + if (!m_doc) { |
| 31 | +- Q_ASSERT(node->type() == QSvgNode::DOC); |
| 32 | +- m_doc = static_cast<QSvgTinyDocument*>(node); |
| 33 | ++ node = method(nullptr, attributes, this); |
| 34 | ++ if (node) { |
| 35 | ++ Q_ASSERT(node->type() == QSvgNode::DOC); |
| 36 | ++ m_doc = static_cast<QSvgTinyDocument*>(node); |
| 37 | ++ } |
| 38 | + } else { |
| 39 | + switch (m_nodes.top()->type()) { |
| 40 | + case QSvgNode::DOC: |
| 41 | +@@ -3742,16 +3743,17 @@ bool QSvgHandler::startElement(const QString &localName, |
| 42 | + case QSvgNode::DEFS: |
| 43 | + case QSvgNode::SWITCH: |
| 44 | + { |
| 45 | +- QSvgStructureNode *group = |
| 46 | +- static_cast<QSvgStructureNode*>(m_nodes.top()); |
| 47 | +- group->addChild(node, someId(attributes)); |
| 48 | ++ node = method(m_nodes.top(), attributes, this); |
| 49 | ++ if (node) { |
| 50 | ++ QSvgStructureNode *group = |
| 51 | ++ static_cast<QSvgStructureNode*>(m_nodes.top()); |
| 52 | ++ group->addChild(node, someId(attributes)); |
| 53 | ++ } |
| 54 | + } |
| 55 | + break; |
| 56 | + default: |
| 57 | + const QByteArray msg = QByteArrayLiteral("Could not add child element to parent element because the types are incorrect."); |
| 58 | + qCWarning(lcSvgHandler, "%s", prefixMessage(msg, xml).constData()); |
| 59 | +- delete node; |
| 60 | +- node = 0; |
| 61 | + break; |
| 62 | + } |
| 63 | + } |
| 64 | +diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp |
| 65 | +index 81c57f7..a0a3d6d 100644 |
| 66 | +--- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp |
| 67 | ++++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp |
| 68 | +@@ -83,6 +83,7 @@ private slots: |
| 69 | + void oss_fuzz_23731(); |
| 70 | + void oss_fuzz_24131(); |
| 71 | + void oss_fuzz_24738(); |
| 72 | ++ void testMisplacedElement(); |
| 73 | + |
| 74 | + #ifndef QT_NO_COMPRESS |
| 75 | + void testGzLoading(); |
| 76 | +@@ -1554,5 +1555,33 @@ void tst_QSvgRenderer::oss_fuzz_24738() |
| 77 | + QSvgRenderer().load(QByteArray("<svg><path d=\"a 2 1e-212.....\">")); |
| 78 | + } |
| 79 | + |
| 80 | ++ |
| 81 | ++void tst_QSvgRenderer::testMisplacedElement() |
| 82 | ++{ |
| 83 | ++ // This input caused a QSvgPattern node to be created with a QSvgPatternStyle referencing to it. |
| 84 | ++ // The code then detected that the <pattern> element is misplaced in the <text> element and |
| 85 | ++ // deleted it. That left behind the QSvgPatternStyle pointing to the deleted QSvgPattern. That |
| 86 | ++ // was reported when running the test with ASAN or UBSAN. |
| 87 | ++ QByteArray svg(R"(<svg> |
| 88 | ++ <text><pattern id="ptn" width="4" height="4"/></text> |
| 89 | ++ <g fill="url(#ptn) "/> |
| 90 | ++ </svg>)"); |
| 91 | ++ |
| 92 | ++ QImage image(20, 20, QImage::Format_ARGB32_Premultiplied); |
| 93 | ++ image.fill(Qt::green); |
| 94 | ++ QImage refImage = image.copy(); |
| 95 | ++ |
| 96 | ++ QTest::ignoreMessage(QtWarningMsg, "<input>:2:68: Could not add child element to parent " |
| 97 | ++ "element because the types are incorrect."); |
| 98 | ++ QTest::ignoreMessage(QtWarningMsg, "<input>:4:28: Could not resolve property: #ptn"); |
| 99 | ++ |
| 100 | ++ QSvgRenderer renderer(svg); |
| 101 | ++ QPainter painter(&image); |
| 102 | ++ renderer.render(&painter); |
| 103 | ++ QCOMPARE(image, refImage); |
| 104 | ++} |
| 105 | ++ |
| 106 | ++} |
| 107 | ++ |
| 108 | + QTEST_MAIN(tst_QSvgRenderer) |
| 109 | + #include "tst_qsvgrenderer.moc" |
| 110 | +-- |
| 111 | +2.45.4 |
| 112 | + |
0 commit comments