CVE-2023-37369
requested to merge asturmlechner/qtbase:asturmlechner/QTBUG-109781-QTBUG-114829-CVE-2023-37369 into kde/5.15
See also: https://www.qt.io/blog/security-advisory-qxmlstreamreader
tl;dr:
- CVE-2023-37369-qtbase-5.15.diff is missing 1a423ce4
- So, cherry-picked missing parent commit, resolved conflicts by looking at upstream patch
- Amended missing chunk so that CVE-2023-37369-qtbase-5.15.diff applied fine in place of 6326bec4, kept tests
Since this turned out to be somewhat of a mess, please take a long hard look at it before merging.
QXmlStreamReader: change fastScanName() to take a Value*
For easier debugging, e.g. to print out value.len and value.prefix.
Pick-to: 6.6 6.5 6.5.2 6.2 5.15
Change-Id: Ib0eed38772f899502962f578775d34ea2744fdde
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
(cherry picked from commit 1a423ce4372d18a779f3c0d746d5283d9a425839)
* asturmlechner 2023-07-09: Fix apparently missing change in
src/corelib/serialization/qxmlstream.g - it caused upstream
CVE-2023-37369-qtbase-5.15.diff to fail to apply (besides missing
this commit in the first place).
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.
Fix typo in QXmlStreamReader error message
Amends 6326bec46a618c72feba4a2bb994c4d475050aed.
Pick-to: 6.6 6.5 6.2 5.15
Task-number: QTBUG-109781
Task-number: QTBUG-114829
Change-Id: Ib5189dc908cd61c6c6fa23024776a4a5baa75ca5
Reviewed-by: Robert Löhning <robert.loehning@qt.io>
(cherry picked from commit bdc8dc51380d2ce4580e6b84e3286ec6f1866156)
tst_QXmlStream: remove unneeded _ba UDLs
... and collapse adjacent C string literals.
Both QStringBuilder and non-QStringBuilder builds have no problem
resolving an operator+ for char[] and QByteArray, so there's no need
to turn the char[] into a QByteArray using the _ba UDL first.
It just causes pain because not all active branches support this UDL,
so remove, to bring this code in line with what the cherry-picks to
6.2 and 5.15 must needs had to use.
Amends 6326bec46a618c72feba4a2bb994c4d475050aed.
Pick-to: 6.6 6.5 6.5.2
Change-Id: Id3d61483729c51c82f58b826efcc8fc7960c3ccd
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
(cherry picked from commit 3bc3b8d69a291aa513d2d120c8ef46f968f1efdf)
Edited by Andreas Sturmlechner