QDom: Guard against null nodes returned by factory functions

Since the default InvalidDataPolicy changed to ReturnNullNode, the
QDomDocumentPrivate factory functions return nullptr when the input
contains data that is invalid per XML 1.0. QDomBuilder::characters(),
QDomBuilder::skippedEntity() and QDomBuilder::comment() used the
returned node without checking, causing a crash.

Check for nullptr and return false, as startElement() and
processingInstruction() already do.

Fixes: QTBUG-148791
Change-Id: Id46c8438e648008d7edad50ae411d682ec5152f6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 8b56704)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit ec02738)
(cherry picked from commit ca6d005)
Reviewed-by: Magdalena Stojek <magdalena.stojek@qt.io>

Author: Magdalenoo

src/xml/dom/qdomhelpers.cpp

@@ -139,6 +139,8 @@ bool QDomBuilder::characters(const QString &characters, bool cdata)
     } else {
         n.reset(doc->createTextNode(characters));
     }
+    if (!n)
+        return false;
     n->setLocation(int(reader->lineNumber()), int(reader->columnNumber()));
     node->appendChild(n.get());
     Q_UNUSED(n.release());
@@ -161,6 +163,8 @@ bool QDomBuilder::processingInstruction(const QString &target, const QString &da
 bool QDomBuilder::skippedEntity(const QString &name)
 {
     QDomNodePrivate *n = doc->createEntityReference(name);
+    if (!n)
+        return false;
     n->setLocation(int(reader->lineNumber()), int(reader->columnNumber()));
     node->appendChild(n);
     return true;
@@ -189,6 +193,8 @@ bool QDomBuilder::comment(const QString &characters)
 {
     QDomNodePrivate *n;
     n = doc->createComment(characters);
+    if (!n)
+        return false;
     n->setLocation(int(reader->lineNumber()), int(reader->columnNumber()));
     node->appendChild(n);
     return true;

tests/auto/xml/dom/qdom/tst_qdom.cpp

@@ -105,6 +105,8 @@ private slots:
     void germanUmlautToFile() const;
     void setInvalidDataPolicy() const;
     void crashInSetContent() const;
+    void invalidCharDataInSetContent() const;
+    void invalidCharDataInSetContent_data() const;
     void doubleNamespaceDeclarations() const;
     void setContentQXmlReaderOverload() const;
     void toStringWithoutNewlines() const;
@@ -2336,6 +2338,60 @@ void tst_QDom::crashInSetContent() const
     QVERIFY(docImport.setContent(QLatin1String("<?xml version=\"1.0\"?><e/>")));
 }
 
+void tst_QDom::invalidCharDataInSetContent_data() const
+{
+    QTest::addColumn<QByteArray>("input");
+    QTest::addColumn<bool>("parsed");
+    QTest::addColumn<QString>("document");
+
+    QTest::newRow("control")
+            << "<r><before/><a>TextNode</a><after/></r>"_ba << true
+            << u"<r><before/><a>TextNode</a><after/></r>"_s;
+    QTest::newRow("text-with-nul")
+            << "<r><before/><a>te\0xt</a><after/></r>"_ba << false
+            << u"<r><before/><a>te</a></r>"_s;
+    QTest::newRow("cdata")
+            << "<r><before/><![CDATA[da\0ta]]><after/></r>"_ba << false
+            << u"<r><before/></r>"_s;
+    QTest::newRow("comment")
+            << "<r><before/><!-- co\0m --><after/></r>"_ba << false
+            << u"<r><before/></r>"_s;
+    QTest::newRow("pi-data")
+            << "<r><before/><?pi da\0ta?><after/></r>"_ba << false
+            << u"<r><before/></r>"_s;
+    QTest::newRow("start-element")
+            << "<r><before/><a\0b/><after/></r>"_ba << false
+            << u"<r><before/></r>"_s;
+    QTest::newRow("nul-at-end")
+            << "<r><before/><before2/></r>\0"_ba << false
+            << u"<r><before/><before2/></r>"_s;
+}
+
+void tst_QDom::invalidCharDataInSetContent() const
+{
+    QFETCH(const QByteArray, input);
+    QFETCH(const bool, parsed);
+    QFETCH(const QString, document);
+
+    const auto policy = QDomImplementation::invalidDataPolicy();
+    const auto restoreInvalidDataPolicy = qScopeGuard([policy] {
+        QDomImplementation::setInvalidDataPolicy(policy);
+    });
+    QDomImplementation::setInvalidDataPolicy(QDomImplementation::ReturnNullNode);
+
+    QDomDocument doc;
+    const QDomDocument::ParseResult result =
+            doc.setContent(input, QDomDocument::ParseOption::UseNamespaceProcessing);
+
+    QCOMPARE(bool(result), parsed);
+    if (!parsed) {
+        QVERIFY(!result.errorMessage.isEmpty());
+        QCOMPARE(result.errorLine, qsizetype(1));
+        QVERIFY(result.errorColumn > 0);
+    }
+    QCOMPARE(doc.toString(-1), document);
+}
+
 void tst_QDom::doubleNamespaceDeclarations() const
 {
     QDomDocument doc;