From 5e3005e2507e6923b61dc5a1b2ceef2c204f0b07 Mon Sep 17 00:00:00 2001
From: Ahmad Samir <a.samirh78@gmail.com>
Date: Thu, 22 Jun 2023 15:56:07 +0300
Subject: [PATCH 100/147] QXmlStreamReader: make fastScanName() indicate
 parsing status to callers

This fixes a crash while parsing an XML file with garbage data, the file
starts with '<' then garbage data:
- The loop in the parse() keeps iterating until it hits "case 262:",
  which calls fastScanName()
- fastScanName() iterates over the text buffer scanning for the
  attribute name (e.g. "xml:lang"), until it finds ':'
- Consider a Value val, fastScanName() is called on it, it would set
  val.prefix to a number > val.len, then it would hit the 4096 condition
  and return (returned 0, now it returns the equivalent of
  std::null_opt), which means that val.len doesn't get modified, making
  it smaller than val.prefix

[cherry-pick comment]
This part was not in upstream's CVE-2023-37369-qtbase-5.15.diff:

- The code would try constructing an XmlStringRef with negative length,
  which would hit an assert in one of QStringView's constructors

Add an assert to the XmlStringRef constructor.
[/cherry-pick comment]

Add unittest based on the file from the bug report.

Later on I will replace FastScanNameResult with std::optional<qsizetype>
(std::optional is C++17, which isn't required by Qt 5.15, and we want to
backport this fix).

Credit to OSS-Fuzz.

Fixes: QTBUG-109781
Fixes: QTBUG-114829
Pick-to: 6.6 6.5 6.2 5.15
Change-Id: I455a5eeb47870c2ac9ffd0cbcdcd99c1ae2dd374
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
(cherry picked from commit 6326bec46a618c72feba4a2bb994c4d475050aed)

* asturmlechner 2023-07-09: This is equivalent to upstream's
  CVE-2023-37369-qtbase-5.15.diff which actually amends 1a423ce4,
  while preserving tests from 6326bec4. Fixed building tests thanks
  to @ahmadsamir.
---
 src/corelib/serialization/qxmlstream.cpp      | 25 ++++++++----
 src/corelib/serialization/qxmlstream.g        | 24 +++++++++--
 src/corelib/serialization/qxmlstream_p.h      | 24 +++++++++--
 .../qxmlstream/tst_qxmlstream.cpp             | 40 +++++++++++++++++++
 4 files changed, 100 insertions(+), 13 deletions(-)

diff --git a/src/corelib/serialization/qxmlstream.cpp b/src/corelib/serialization/qxmlstream.cpp
index 758cf9ba9b..561f3107a9 100644
--- a/src/corelib/serialization/qxmlstream.cpp
+++ b/src/corelib/serialization/qxmlstream.cpp
@@ -1302,15 +1302,18 @@ inline int QXmlStreamReaderPrivate::fastScanContentCharList()
     return n;
 }
 
-inline int QXmlStreamReaderPrivate::fastScanName(Value *val)
+// Fast scan an XML attribute name (e.g. "xml:lang").
+inline QXmlStreamReaderPrivate::FastScanNameResult
+QXmlStreamReaderPrivate::fastScanName(Value *val)
 {
     int n = 0;
     uint c;
     while ((c = getChar()) != StreamEOF) {
         if (n >= 4096) {
             // This is too long to be a sensible name, and
-            // can exhaust memory
-            return 0;
+            // can exhaust memory, or the range of decltype(*prefix)
+            raiseNamePrefixTooLongError();
+            return {};
         }
         switch (c) {
         case '\n':
@@ -1344,18 +1347,18 @@ inline int QXmlStreamReaderPrivate::fastScanName(Value *val)
                 putChar(':');
                 --n;
             }
-            return n;
+            return FastScanNameResult(n);
         case ':':
             if (val) {
                 if (val->prefix == 0) {
                     val->prefix = n + 2;
                 } else { // only one colon allowed according to the namespace spec.
                     putChar(c);
-                    return n;
+                    return FastScanNameResult(n);
                 }
             } else {
                 putChar(c);
-                return n;
+                return FastScanNameResult(n);
             }
             Q_FALLTHROUGH();
         default:
@@ -1369,7 +1372,7 @@ inline int QXmlStreamReaderPrivate::fastScanName(Value *val)
     int pos = textBuffer.size() - n;
     putString(textBuffer, pos);
     textBuffer.resize(pos);
-    return 0;
+    return FastScanNameResult(0);
 }
 
 enum NameChar { NameBeginning, NameNotBeginning, NotName };
@@ -1878,6 +1881,14 @@ void QXmlStreamReaderPrivate::raiseWellFormedError(const QString &message)
     raiseError(QXmlStreamReader::NotWellFormedError, message);
 }
 
+void QXmlStreamReaderPrivate::raiseNamePrefixTooLongError()
+{
+    // TODO: add a ImplementationLimitsExceededError and use it instead
+    raiseError(QXmlStreamReader::NotWellFormedError,
+               QXmlStream::tr("Length of XML attribute name exceeds implemnetation limits (4KiB "
+                              "characters)."));
+}
+
 void QXmlStreamReaderPrivate::parseError()
 {
 
diff --git a/src/corelib/serialization/qxmlstream.g b/src/corelib/serialization/qxmlstream.g
index 83e18de6b0..8c6a1a5887 100644
--- a/src/corelib/serialization/qxmlstream.g
+++ b/src/corelib/serialization/qxmlstream.g
@@ -516,7 +516,16 @@ public:
     int fastScanLiteralContent();
     int fastScanSpace();
     int fastScanContentCharList();
-    int fastScanName(Value *val = nullptr);
+
+    struct FastScanNameResult {
+        FastScanNameResult() : ok(false) {}
+        explicit FastScanNameResult(int len) : addToLen(len), ok(true) { }
+        operator bool() { return ok; }
+        int operator*() { Q_ASSERT(ok); return addToLen; }
+        int addToLen;
+        bool ok;
+    };
+    FastScanNameResult fastScanName(Value *val = nullptr);
     inline int fastScanNMTOKEN();
 
 
@@ -525,6 +534,7 @@ public:
 
     void raiseError(QXmlStreamReader::Error error, const QString& message = QString());
     void raiseWellFormedError(const QString &message);
+    void raiseNamePrefixTooLongError();
 
     QXmlStreamEntityResolver *entityResolver;
 
@@ -1812,7 +1822,11 @@ qname ::= LETTER;
 /.
         case $rule_number: {
             Value &val = sym(1);
-            val.len += fastScanName(&val);
+            if (auto res = fastScanName(&val))
+                val.len += *res;
+            else
+                return false;
+
             if (atEnd) {
                 resume($rule_number);
                 return false;
@@ -1823,7 +1837,11 @@ qname ::= LETTER;
 name ::= LETTER;
 /.
         case $rule_number:
-            sym(1).len += fastScanName();
+            if (auto res = fastScanName())
+                sym(1).len += *res;
+            else
+                return false;
+
             if (atEnd) {
                 resume($rule_number);
                 return false;
diff --git a/src/corelib/serialization/qxmlstream_p.h b/src/corelib/serialization/qxmlstream_p.h
index 7c7dbb45dc..b01484cac3 100644
--- a/src/corelib/serialization/qxmlstream_p.h
+++ b/src/corelib/serialization/qxmlstream_p.h
@@ -1005,7 +1005,16 @@ public:
     int fastScanLiteralContent();
     int fastScanSpace();
     int fastScanContentCharList();
-    int fastScanName(Value *val = nullptr);
+
+    struct FastScanNameResult {
+        FastScanNameResult() : ok(false) {}
+        explicit FastScanNameResult(int len) : addToLen(len), ok(true) { }
+        operator bool() { return ok; }
+        int operator*() { Q_ASSERT(ok); return addToLen; }
+        int addToLen;
+        bool ok;
+    };
+    FastScanNameResult fastScanName(Value *val = nullptr);
     inline int fastScanNMTOKEN();
 
 
@@ -1014,6 +1023,7 @@ public:
 
     void raiseError(QXmlStreamReader::Error error, const QString& message = QString());
     void raiseWellFormedError(const QString &message);
+    void raiseNamePrefixTooLongError();
 
     QXmlStreamEntityResolver *entityResolver;
 
@@ -1940,7 +1950,11 @@ bool QXmlStreamReaderPrivate::parse()
 
         case 262: {
             Value &val = sym(1);
-            val.len += fastScanName(&val);
+            if (auto res = fastScanName(&val))
+                val.len += *res;
+            else
+                return false;
+
             if (atEnd) {
                 resume(262);
                 return false;
@@ -1948,7 +1962,11 @@ bool QXmlStreamReaderPrivate::parse()
         } break;
 
         case 263:
-            sym(1).len += fastScanName();
+            if (auto res = fastScanName())
+                sym(1).len += *res;
+            else
+                return false;
+
             if (atEnd) {
                 resume(263);
                 return false;
diff --git a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp
index 533fcd96f5..31a127f0de 100644
--- a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp
+++ b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp
@@ -42,6 +42,7 @@
 
 #include "qc14n.h"
 
+Q_DECLARE_METATYPE(QXmlStreamReader::Error)
 Q_DECLARE_METATYPE(QXmlStreamReader::ReadElementTextBehaviour)
 
 static const char *const catalogFile = "XML-Test-Suite/xmlconf/finalCatalog.xml";
@@ -587,6 +588,8 @@ private slots:
     void readBack() const;
     void roundTrip() const;
     void roundTrip_data() const;
+    void test_fastScanName_data() const;
+    void test_fastScanName() const;
 
     void entityExpansionLimit() const;
 
@@ -1842,5 +1845,42 @@ void tst_QXmlStream::roundTrip() const
     QCOMPARE(out, in);
 }
 
+void tst_QXmlStream::test_fastScanName_data() const
+{
+    QTest::addColumn<QByteArray>("data");
+    QTest::addColumn<QXmlStreamReader::Error>("errorType");
+
+    // 4096 is the limit in QXmlStreamReaderPrivate::fastScanName()
+
+    QByteArray arr = "<a"_ba + ":" + QByteArray("b").repeated(4096 - 1);
+    QTest::newRow("data1") << arr << QXmlStreamReader::PrematureEndOfDocumentError;
+
+    arr = "<a"_ba + ":" + QByteArray("b").repeated(4096);
+    QTest::newRow("data2") << arr << QXmlStreamReader::NotWellFormedError;
+
+    arr = "<"_ba + QByteArray("a").repeated(4000) + ":" + QByteArray("b").repeated(96);
+    QTest::newRow("data3") << arr << QXmlStreamReader::PrematureEndOfDocumentError;
+
+    arr = "<"_ba + QByteArray("a").repeated(4000) + ":" + QByteArray("b").repeated(96 + 1);
+    QTest::newRow("data4") << arr << QXmlStreamReader::NotWellFormedError;
+
+    arr = "<"_ba + QByteArray("a").repeated(4000 + 1) + ":" + QByteArray("b").repeated(96);
+    QTest::newRow("data5") << arr << QXmlStreamReader::NotWellFormedError;
+}
+
+void tst_QXmlStream::test_fastScanName() const
+{
+    QFETCH(QByteArray, data);
+    QFETCH(QXmlStreamReader::Error, errorType);
+
+    QXmlStreamReader reader(data);
+    QXmlStreamReader::TokenType tokenType;
+    while (!reader.atEnd())
+        tokenType = reader.readNext();
+
+    QCOMPARE(tokenType, QXmlStreamReader::Invalid);
+    QCOMPARE(reader.error(), errorType);
+}
+
 #include "tst_qxmlstream.moc"
 // vim: et:ts=4:sw=4:sts=4
-- 
2.40.1