Skip to content

CVE-2023-37369

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

Merge request reports