From bf1012ecb1621f4c60328f3bd88b03f30775db08 Mon Sep 17 00:00:00 2001 From: Patrick Franz Date: Fri, 22 Dec 2023 16:11:30 +0100 Subject: [PATCH] Add patch to fix CVE-2023-37369. --- debian/changelog | 3 + debian/patches/cve-2023-37369.diff | 284 +++++++++++++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 288 insertions(+) create mode 100644 debian/patches/cve-2023-37369.diff diff --git a/debian/changelog b/debian/changelog index d82213e..f83f245 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,5 +1,8 @@ qt6-base (6.4.2+dfsg-20) UNRELEASED; urgency=medium + [ Patrick Franz ] + * Add patch to fix CVE-2023-37369. + -- Debian Qt/KDE Maintainers Fri, 22 Dec 2023 15:10:10 +0100 qt6-base (6.4.2+dfsg-19) unstable; urgency=medium diff --git a/debian/patches/cve-2023-37369.diff b/debian/patches/cve-2023-37369.diff new file mode 100644 index 0000000..586b188 --- /dev/null +++ b/debian/patches/cve-2023-37369.diff @@ -0,0 +1,284 @@ +From 6326bec46a618c72feba4a2bb994c4d475050aed Mon Sep 17 00:00:00 2001 +From: Ahmad Samir +Date: Thu, 22 Jun 2023 15:56:07 +0300 +Subject: [PATCH] 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 +- 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. + +Add unittest based on the file from the bug report. + +Later on I will replace FastScanNameResult with std::optional +(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 +--- + +diff --git a/src/corelib/serialization/qxmlstream.cpp b/src/corelib/serialization/qxmlstream.cpp +index b76243c176..adb57a0239 100644 +--- a/src/corelib/serialization/qxmlstream.cpp ++++ b/src/corelib/serialization/qxmlstream.cpp +@@ -1243,7 +1243,10 @@ inline qsizetype QXmlStreamReaderPrivate::fastScanContentCharList() + return n; + } + +-inline qsizetype QXmlStreamReaderPrivate::fastScanName(qint16 *prefix) ++// Fast scan an XML attribute name (e.g. "xml:lang"). ++inline QXmlStreamReaderPrivate::FastScanNameResult ++QXmlStreamReaderPrivate::fastScanName(Value *val) ++ + { + qsizetype n = 0; + uint c; +@@ -1251,7 +1254,8 @@ inline qsizetype QXmlStreamReaderPrivate::fastScanName(qint16 *prefix) + if (n >= 4096) { + // This is too long to be a sensible name, and + // can exhaust memory, or the range of decltype(*prefix) +- return 0; ++ raiseNamePrefixTooLongError(); ++ return {}; + } + switch (c) { + case '\n': +@@ -1280,23 +1284,24 @@ inline qsizetype QXmlStreamReaderPrivate::fastScanName(qint16 *prefix) + case '+': + case '*': + putChar(c); +- if (prefix && *prefix == n+1) { +- *prefix = 0; ++ if (val && val->prefix == n + 1) { ++ val->prefix = 0; ++ + putChar(':'); + --n; + } +- return n; ++ return FastScanNameResult(n); + case ':': +- if (prefix) { +- if (*prefix == 0) { +- *prefix = qint16(n + 2); ++ 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: +@@ -1305,12 +1310,13 @@ inline qsizetype QXmlStreamReaderPrivate::fastScanName(qint16 *prefix) + } + } + +- if (prefix) +- *prefix = 0; ++ if (val) ++ val->prefix = 0; ++ + qsizetype pos = textBuffer.size() - n; + putString(textBuffer, pos); + textBuffer.resize(pos); +- return 0; ++ return FastScanNameResult(0); + } + + enum NameChar { NameBeginning, NameNotBeginning, NotName }; +@@ -1791,6 +1797,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 d06c371eb8..fc122e6681 100644 +--- a/src/corelib/serialization/qxmlstream.g ++++ b/src/corelib/serialization/qxmlstream.g +@@ -1419,7 +1419,12 @@ space_opt ::= space; + qname ::= LETTER; + /. + case $rule_number: { +- sym(1).len += fastScanName(&sym(1).prefix); ++ Value &val = sym(1); ++ if (auto res = fastScanName(&val)) ++ val.len += *res; ++ else ++ return false; ++ + if (atEnd) { + resume($rule_number); + return false; +@@ -1430,7 +1435,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 1fd69a2c1f..fba6aeb496 100644 +--- a/src/corelib/serialization/qxmlstream_p.h ++++ b/src/corelib/serialization/qxmlstream_p.h +@@ -38,7 +38,7 @@ public: + + constexpr XmlStringRef() = default; + constexpr inline XmlStringRef(const QString *string, qsizetype pos, qsizetype length) +- : m_string(string), m_pos(pos), m_size(length) ++ : m_string(string), m_pos(pos), m_size((Q_ASSERT(length >= 0), length)) + { + } + XmlStringRef(const QString *string) +@@ -471,7 +471,16 @@ public: + qsizetype fastScanLiteralContent(); + qsizetype fastScanSpace(); + qsizetype fastScanContentCharList(); +- qsizetype fastScanName(qint16 *prefix = nullptr); ++ ++ struct FastScanNameResult { ++ FastScanNameResult() : ok(false) {} ++ explicit FastScanNameResult(qsizetype len) : addToLen(len), ok(true) { } ++ operator bool() { return ok; } ++ qsizetype operator*() { Q_ASSERT(ok); return addToLen; } ++ qsizetype addToLen; ++ bool ok; ++ }; ++ FastScanNameResult fastScanName(Value *val = nullptr); + inline qsizetype fastScanNMTOKEN(); + + +@@ -480,6 +489,7 @@ public: + + void raiseError(QXmlStreamReader::Error error, const QString& message = QString()); + void raiseWellFormedError(const QString &message); ++ void raiseNamePrefixTooLongError(); + + QXmlStreamEntityResolver *entityResolver; + +diff --git a/src/corelib/serialization/qxmlstreamparser_p.h b/src/corelib/serialization/qxmlstreamparser_p.h +index e3ae6faa44..afd83381b3 100644 +--- a/src/corelib/serialization/qxmlstreamparser_p.h ++++ b/src/corelib/serialization/qxmlstreamparser_p.h +@@ -947,7 +947,12 @@ bool QXmlStreamReaderPrivate::parse() + break; + + case 262: { +- sym(1).len += fastScanName(&sym(1).prefix); ++ Value &val = sym(1); ++ if (auto res = fastScanName(&val)) ++ val.len += *res; ++ else ++ return false; ++ + if (atEnd) { + resume(262); + return false; +@@ -955,7 +960,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 2799e7a999..7eb0aac5cc 100644 +--- a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp ++++ b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp +@@ -581,6 +581,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; + +@@ -1753,5 +1755,42 @@ void tst_QXmlStream::roundTrip() const + QCOMPARE(out, in); + } + ++void tst_QXmlStream::test_fastScanName_data() const ++{ ++ QTest::addColumn("data"); ++ QTest::addColumn("errorType"); ++ ++ // 4096 is the limit in QXmlStreamReaderPrivate::fastScanName() ++ ++ QByteArray arr = "