qtbase-opensource-src/debian/patches/CVE-2023-37369.diff

309 lines
11 KiB
Diff

From: Debian Qt/KDE Maintainers <debian-qt-kde@lists.debian.org>
Date: Mon, 20 Nov 2023 01:57:45 +0000
Subject: QXmlStreamReader: make fastScanName() indicate parsing status to
callers
Origin: upstream, commits
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=1a423ce4372d18a7
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=6326bec46a618c72
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=bdc8dc51380d2ce4
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=3bc3b8d69a291aa5
.
Based on KDE's backport:
https://invent.kde.org/qt/qt/qtbase/-/merge_requests/263
Last-Update: 2023-07-15
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.
Credit to OSS-Fuzz.
---
src/corelib/serialization/qxmlstream.cpp | 39 +++++++++++++--------
src/corelib/serialization/qxmlstream.g | 25 ++++++++++++--
src/corelib/serialization/qxmlstream_p.h | 25 ++++++++++++--
.../serialization/qxmlstream/tst_qxmlstream.cpp | 40 ++++++++++++++++++++++
4 files changed, 109 insertions(+), 20 deletions(-)
diff --git a/src/corelib/serialization/qxmlstream.cpp b/src/corelib/serialization/qxmlstream.cpp
index 7cd457b..7b4b223 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(int *prefix)
+// 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':
@@ -1339,23 +1342,23 @@ inline int QXmlStreamReaderPrivate::fastScanName(int *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 = 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:
@@ -1364,12 +1367,12 @@ inline int QXmlStreamReaderPrivate::fastScanName(int *prefix)
}
}
- if (prefix)
- *prefix = 0;
+ if (val)
+ val->prefix = 0;
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 implementation limits (4KiB "
+ "characters)."));
+}
+
void QXmlStreamReaderPrivate::parseError()
{
diff --git a/src/corelib/serialization/qxmlstream.g b/src/corelib/serialization/qxmlstream.g
index 4321fed..8c6a1a5 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(int *prefix = 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;
@@ -1811,7 +1821,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;
@@ -1822,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 e5bde7b..b01484c 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(int *prefix = 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;
@@ -1939,7 +1949,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;
@@ -1947,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 533fcd9..513ba9b 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:" + QByteArray("b").repeated(4096 - 1);
+ QTest::newRow("data1") << arr << QXmlStreamReader::PrematureEndOfDocumentError;
+
+ arr = "<a:" + QByteArray("b").repeated(4096);
+ QTest::newRow("data2") << arr << QXmlStreamReader::NotWellFormedError;
+
+ arr = "<" + QByteArray("a").repeated(4000) + ":" + QByteArray("b").repeated(96);
+ QTest::newRow("data3") << arr << QXmlStreamReader::PrematureEndOfDocumentError;
+
+ arr = "<" + QByteArray("a").repeated(4000) + ":" + QByteArray("b").repeated(96 + 1);
+ QTest::newRow("data4") << arr << QXmlStreamReader::NotWellFormedError;
+
+ arr = "<" + 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