From 474f268a8e04a4d2d66aa7e36b3b72cc1c1ef769 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Mon, 01 Jun 2026 17:49:07 +0200
Subject: [PATCH] QDomNode: fix unbounded nesting depth on destruction and clear()

In both ~QDomNodePrivate() and QDomNodePrivate::clear(), the code
simply walked the list of chldren (->first, ->next), and deleted each
one in turn. This recurses into ~QDomNodePrivate() and uses stack
space proportional to the height of the tree. Since the latter is
user-controlled, this is a an easy DoS, so fix the implementation to
use iteration instead of recursion.

Note that it suffices to fix ~QDomNodePrivate(), as clear() only
recurses via the dtor, too.

Found by yours truly, confirmed by Claude Opus 4.7. Fix vibe-coded
with Opus 4.8.

The code is well-commented, so I won't spill a lot of ink in the
commit message, but the general idea is to perform a post-order
depth-first-search where visitation means deleting. Since we visited
all children before we visit their parent, we only ever delete empty
nodes, which won't recurse. In particular, this doesn't change the
order or deletion from that of the old code, when calculated as end
of dtor (of course, since we remove recursion, the entering order of
dtors changes).

Add tests that exhaust the stack even on Linux (8MiB) before the
fix and execute in less than 100ms after. While writing the test,
found QTBUG-147190... :(

The tests are tiny bit DRYer than I'd otherwise shoot for, since
I plan to make the toByteArray() deep-nesting check use the same
framework.

Amends the start of the public history.

Fixes: QTBUG-147191
Pick-to: 6.8 6.5 5.15
Change-Id: I154a1db71518b037c296fc5f993599f106296e81
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
(cherry picked from commit 647b221ca885739a69ece5695b7dcddcf3227558)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit ea71bb878603a0f1dd954d1a473c92967d6b2aec)
---

diff --git a/src/xml/dom/qdom.cpp b/src/xml/dom/qdom.cpp
index c58a1e3..9ff8da2 100644
--- a/src/xml/dom/qdom.cpp
+++ b/src/xml/dom/qdom.cpp
@@ -1086,16 +1086,21 @@
 
 QDomNodePrivate::~QDomNodePrivate()
 {
-    QDomNodePrivate* p = first;
-    QDomNodePrivate* n;
+    QDomNodePrivate *p = this;
 
-    while (p) {
-        n = p->next;
-        if (!p->ref.deref())
-            delete p;
-        else
-            p->setNoParent();
-        p = n;
+    // post-order depth-first-search; visitation is deletion (avoids recursion)
+    while (true) {
+        if (QDomNodePrivate *c = p->first) {
+            p->first = c->next;          // peel firstChild off p
+            if (c->ref.deref())
+                c->setNoParent();        // survivor: detach, don't descend
+            else
+                p = c;                   // descend; c's parent() remembers p
+        } else {                         // p ran out of children (= is a leaf now)
+            if (p == this)
+                break;                   // we're done, don't `delete this`
+            delete std::exchange(p, p->parent());  // deletes and ascends
+        }
     }
 }
 
diff --git a/tests/auto/xml/dom/qdom/tst_qdom.cpp b/tests/auto/xml/dom/qdom/tst_qdom.cpp
index 2c62e6f..0c68221 100644
--- a/tests/auto/xml/dom/qdom/tst_qdom.cpp
+++ b/tests/auto/xml/dom/qdom/tst_qdom.cpp
@@ -10,6 +10,7 @@
 #include <QtCore/qbytearray.h>
 #include <QtCore/qcoreapplication.h>
 #include <QtCore/qdebug.h>
+#include <QtCore/qelapsedtimer.h>
 #include <QtCore/qfile.h>
 #include <QtCore/qlist.h>
 #include <QtCore/qregularexpression.h>
@@ -126,6 +127,8 @@
     void testDomListComparison_data();
     void testDomListComparison();
     void noCrashOnDeepNesting() const;
+    void noCrashOnDeepNestingDtor() const { noCrashOnDeepNesting_impl(DeepNestingOp::Dtor); }
+    void noCrashOnDeepNestingClear() const { noCrashOnDeepNesting_impl(DeepNestingOp::Clear); }
 
     void cleanupTestCase() const;
 
@@ -138,6 +141,12 @@
     static QString onNullWarning(const char *const functionName);
     static bool isDeepEqual(const QDomNode &n1, const QDomNode &n2);
     static bool isFakeXMLDeclaration(const QDomNode &node);
+    enum class DeepNestingOp {
+        Dtor,
+        Clear,
+    };
+    static constexpr size_t DeepNestingDepth = 250'000;
+    static void noCrashOnDeepNesting_impl(DeepNestingOp op);
 
     QList<QByteArray> m_testCodecs;
 };
@@ -2716,6 +2725,25 @@
     QT_TEST_EQUALITY_OPS(lhs, rhs, result);
 }
 
+static QDomDocument makeNested(size_t depth)
+{
+    QDomDocument doc;
+    if (!depth)
+        return doc;
+
+    // Need to build it bottom-up (QTBUG-147190)...
+
+    QDomElement a = doc.createElement("a");
+    for (size_t i = 1; i < depth; ++i) {
+        QDomElement parent = doc.createElement("a");
+        parent.appendChild(std::exchange(a, parent));
+    }
+
+    doc.appendChild(a); // only now associate it with the doc (QTBUG-147190)
+
+    return doc;
+}
+
 // The fix of QTBUG-131151 crash
 void tst_QDom::noCrashOnDeepNesting() const
 {
@@ -2732,5 +2760,35 @@
     file.close();
 }
 
+// QTBUG-146936: dtor
+void tst_QDom::noCrashOnDeepNesting_impl(DeepNestingOp op)
+{
+    const auto print = [](const QElapsedTimer &timer, const char *operation) {
+        std::chrono::duration<double> secs = timer.durationElapsed();
+        qDebug("%s %llu-deep document in %fs", operation,
+               qulonglong(DeepNestingDepth), secs.count());
+    };
+
+    QElapsedTimer timer;
+    timer.start();
+    std::optional doc = makeNested(DeepNestingDepth);
+    print(timer, "created");
+
+    timer.restart();
+    const char *operation;
+    // the actual test is that it doesn't run off the stack and crashes (QTBUG-146936):
+    switch (op) {
+    case DeepNestingOp::Dtor:
+        operation = "destroyed";
+        doc.reset();
+        break;
+    case DeepNestingOp::Clear:
+        operation = "cleared";
+        doc->clear();
+        break;
+    }
+    print(timer, operation);
+}
+
 QTEST_MAIN(tst_QDom)
 #include "tst_qdom.moc"