Fix CVE-2024-25580: potential buffer overflow when reading KTX images

Resolves: RHEL-20239

.qt5-qtbase.metadata

@@ -0,0 +1,2 @@
+a5bbeafa6319cd3e666b12ccc722a357de7230be qtbase-everywhere-opensource-src-5.15.9.tar.xz
+677b605bf6033bdfa84a676096ec6e77da6e844d kde-5.15-rollup-20230411.patch.gz

0001-CVE-2023-51714-qtbase-5.15.patch

@@ -0,0 +1,38 @@
+From ea63c28efc1d2ecb467b83a34923d12462efa96f Mon Sep 17 00:00:00 2001
+From: Marc Mutz <marc.mutz@qt.io>
+Date: Tue, 12 Dec 2023 20:51:56 +0100
+Subject: [PATCH] HPack: fix a Yoda Condition
+
+Putting the variable on the LHS of a relational operation makes the
+expression easier to read. In this case, we find that the whole
+expression is nonsensical as an overflow protection, because if
+name.size() + value.size() overflows, the result will exactly _not_
+be > max() - 32, because UB will have happened.
+
+To be fixed in a follow-up commit.
+
+As a drive-by, add parentheses around the RHS.
+
+Change-Id: I35ce598884c37c51b74756b3bd2734b9aad63c09
+Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
+(cherry picked from commit 658607a34ead214fbacbc2cca44915655c318ea9)
+Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
+(cherry picked from commit 4f7efd41740107f90960116700e3134f5e433867)
+(cherry picked from commit 13c16b756900fe524f6d9534e8a07aa003c05e0c)
+(cherry picked from commit 1d4788a39668fb2dc5912a8d9c4272dc40e99f92)
+(cherry picked from commit 87de75b5cc946d196decaa6aef4792a6cac0b6db)
+---
+
+diff --git a/src/network/access/http2/hpacktable.cpp b/src/network/access/http2/hpacktable.cpp
+index 834214f..ab166a6 100644
+--- a/src/network/access/http2/hpacktable.cpp
++++ b/src/network/access/http2/hpacktable.cpp
+@@ -63,7 +63,7 @@
+     // 32 octets of overhead."
+
+     const unsigned sum = unsigned(name.size() + value.size());
+-    if (std::numeric_limits<unsigned>::max() - 32 < sum)
++    if (sum > (std::numeric_limits<unsigned>::max() - 32))
+         return HeaderSize();
+     return HeaderSize(true, quint32(sum + 32));
+ }

0002-CVE-2023-51714-qtbase-5.15.patch

@@ -0,0 +1,59 @@
+From 23c3fc483e8b6e21012a61f0bea884446f727776 Mon Sep 17 00:00:00 2001
+From: Marc Mutz <marc.mutz@qt.io>
+Date: Tue, 12 Dec 2023 22:08:07 +0100
+Subject: [PATCH] HPack: fix incorrect integer overflow check
+
+This code never worked:
+
+For the comparison with max() - 32 to trigger, on 32-bit platforms (or
+Qt 5) signed interger overflow would have had to happen in the
+addition of the two sizes. The compiler can therefore remove the
+overflow check as dead code.
+
+On Qt 6 and 64-bit platforms, the signed integer addition would be
+very unlikely to overflow, but the following truncation to uint32
+would yield the correct result only in a narrow 32-value window just
+below UINT_MAX, if even that.
+
+Fix by using the proper tool, qAddOverflow.
+
+Manual conflict resolutions:
+ - qAddOverflow doesn't exist in Qt 5, use private add_overflow
+   predecessor API instead
+
+Change-Id: I7599f2e75ff7f488077b0c60b81022591005661c
+Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
+(cherry picked from commit ee5da1f2eaf8932aeca02ffea6e4c618585e29e3)
+Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
+(cherry picked from commit debeb8878da2dc706ead04b6072ecbe7e5313860)
+Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
+Reviewed-by: Marc Mutz <marc.mutz@qt.io>
+(cherry picked from commit 811b9eef6d08d929af8708adbf2a5effb0eb62d7)
+(cherry picked from commit f931facd077ce945f1e42eaa3bead208822d3e00)
+(cherry picked from commit 9ef4ca5ecfed771dab890856130e93ef5ceabef5)
+Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io>
+---
+
+diff --git a/src/network/access/http2/hpacktable.cpp b/src/network/access/http2/hpacktable.cpp
+index ab166a6..de91fc0 100644
+--- a/src/network/access/http2/hpacktable.cpp
++++ b/src/network/access/http2/hpacktable.cpp
+@@ -40,6 +40,7 @@
+ #include "hpacktable_p.h"
+
+ #include <QtCore/qdebug.h>
++#include <QtCore/private/qnumeric_p.h>
+
+ #include <algorithm>
+ #include <cstddef>
+@@ -62,7 +63,9 @@
+     // for counting the number of references to the name and value would have
+     // 32 octets of overhead."
+
+-    const unsigned sum = unsigned(name.size() + value.size());
++    size_t sum;
++    if (add_overflow(size_t(name.size()), size_t(value.size()), &sum))
++        return HeaderSize();
+     if (sum > (std::numeric_limits<unsigned>::max() - 32))
+         return HeaderSize();
+     return HeaderSize(true, quint32(sum + 32));

CVE-2024-25580-qtbase-5.15.patch

@@ -0,0 +1,197 @@
+diff --git a/src/gui/util/qktxhandler.cpp b/src/gui/util/qktxhandler.cpp
+index 0d98e97453..6a79e55109 100644
+--- a/src/gui/util/qktxhandler.cpp
++++ b/src/gui/util/qktxhandler.cpp
+@@ -73,7 +73,7 @@ struct KTXHeader {
+     quint32 bytesOfKeyValueData;
+ };
+
+-static const quint32 headerSize = sizeof(KTXHeader);
++static constexpr quint32 qktxh_headerSize = sizeof(KTXHeader);
+
+ // Currently unused, declared for future reference
+ struct KTXKeyValuePairItem {
+@@ -103,11 +103,36 @@ struct KTXMipmapLevel {
+     */
+ };
+
+-bool QKtxHandler::canRead(const QByteArray &suffix, const QByteArray &block)
++static bool qAddOverflow(quint32 v1, quint32 v2, quint32 *r) {
++    // unsigned additions are well-defined
++    *r = v1 + v2;
++    return v1 > quint32(v1 + v2);
++}
++
++// Returns the nearest multiple of 4 greater than or equal to 'value'
++static bool nearestMultipleOf4(quint32 value, quint32 *result)
++{
++    constexpr quint32 rounding = 4;
++    *result = 0;
++    if (qAddOverflow(value, rounding - 1, result))
++        return true;
++    *result &= ~(rounding - 1);
++    return false;
++}
++
++// Returns a slice with prechecked bounds
++static QByteArray safeSlice(const QByteArray& array, quint32 start, quint32 length)
+ {
+-    Q_UNUSED(suffix)
++    quint32 end = 0;
++    if (qAddOverflow(start, length, &end) || end > quint32(array.length()))
++        return {};
++    return QByteArray(array.data() + start, length);
++}
+
+-    return (qstrncmp(block.constData(), ktxIdentifier, KTX_IDENTIFIER_LENGTH) == 0);
++bool QKtxHandler::canRead(const QByteArray &suffix, const QByteArray &block)
++{
++    Q_UNUSED(suffix);
++    return block.startsWith(QByteArray::fromRawData(ktxIdentifier, KTX_IDENTIFIER_LENGTH));
+ }
+
+ QTextureFileData QKtxHandler::read()
+@@ -115,42 +140,97 @@ QTextureFileData QKtxHandler::read()
+     if (!device())
+         return QTextureFileData();
+
+-    QByteArray buf = device()->readAll();
+-    const quint32 dataSize = quint32(buf.size());
+-    if (dataSize < headerSize || !canRead(QByteArray(), buf)) {
+-        qCDebug(lcQtGuiTextureIO, "Invalid KTX file %s", logName().constData());
++    const QByteArray buf = device()->readAll();
++    if (size_t(buf.size()) > std::numeric_limits<quint32>::max()) {
++        qWarning(lcQtGuiTextureIO, "Too big KTX file %s", logName().constData());
++        return QTextureFileData();
++    }
++
++    if (!canRead(QByteArray(), buf)) {
++        qWarning(lcQtGuiTextureIO, "Invalid KTX file %s", logName().constData());
++        return QTextureFileData();
++    }
++
++    if (buf.size() < qsizetype(qktxh_headerSize)) {
++        qWarning(lcQtGuiTextureIO, "Invalid KTX header size in %s", logName().constData());
+         return QTextureFileData();
+     }
+
+-    const KTXHeader *header = reinterpret_cast<const KTXHeader *>(buf.constData());
+-    if (!checkHeader(*header)) {
+-        qCDebug(lcQtGuiTextureIO, "Unsupported KTX file format in %s", logName().constData());
++    KTXHeader header;
++    memcpy(&header, buf.data(), qktxh_headerSize);
++    if (!checkHeader(header)) {
++        qWarning(lcQtGuiTextureIO, "Unsupported KTX file format in %s", logName().constData());
+         return QTextureFileData();
+     }
+
+     QTextureFileData texData;
+     texData.setData(buf);
+
+-    texData.setSize(QSize(decode(header->pixelWidth), decode(header->pixelHeight)));
+-    texData.setGLFormat(decode(header->glFormat));
+-    texData.setGLInternalFormat(decode(header->glInternalFormat));
+-    texData.setGLBaseInternalFormat(decode(header->glBaseInternalFormat));
+-
+-    texData.setNumLevels(decode(header->numberOfMipmapLevels));
+-    quint32 offset = headerSize + decode(header->bytesOfKeyValueData);
+-    const int maxLevels = qMin(texData.numLevels(), 32);               // Cap iterations in case of corrupt file.
+-    for (int i = 0; i < maxLevels; i++) {
+-        if (offset + sizeof(KTXMipmapLevel) > dataSize)                // Corrupt file; avoid oob read
+-            break;
+-        const KTXMipmapLevel *level = reinterpret_cast<const KTXMipmapLevel *>(buf.constData() + offset);
+-        quint32 levelLen = decode(level->imageSize);
+-        texData.setDataOffset(offset + sizeof(KTXMipmapLevel::imageSize), i);
+-        texData.setDataLength(levelLen, i);
+-        offset += sizeof(KTXMipmapLevel::imageSize) + levelLen + (3 - ((levelLen + 3) % 4));
++    texData.setSize(QSize(decode(header.pixelWidth), decode(header.pixelHeight)));
++    texData.setGLFormat(decode(header.glFormat));
++    texData.setGLInternalFormat(decode(header.glInternalFormat));
++    texData.setGLBaseInternalFormat(decode(header.glBaseInternalFormat));
++
++    texData.setNumLevels(decode(header.numberOfMipmapLevels));
++
++    const quint32 bytesOfKeyValueData = decode(header.bytesOfKeyValueData);
++    quint32 headerKeyValueSize;
++    if (qAddOverflow(qktxh_headerSize, bytesOfKeyValueData, &headerKeyValueSize)) {
++        qWarning(lcQtGuiTextureIO, "Overflow in size of key value data in header of KTX file %s",
++                 logName().constData());
++        return QTextureFileData();
++    }
++
++    if (headerKeyValueSize >= quint32(buf.size())) {
++        qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData());
++        return QTextureFileData();
++    }
++
++    // Technically, any number of levels is allowed but if the value is bigger than
++    // what is possible in KTX V2 (and what makes sense) we return an error.
++    // maxLevels = log2(max(width, height, depth))
++    const int maxLevels = (sizeof(quint32) * 8)
++            - qCountLeadingZeroBits(std::max(
++                    { header.pixelWidth, header.pixelHeight, header.pixelDepth }));
++
++    if (texData.numLevels() > maxLevels) {
++        qWarning(lcQtGuiTextureIO, "Too many levels in KTX file %s", logName().constData());
++        return QTextureFileData();
++    }
++
++    quint32 offset = headerKeyValueSize;
++    for (int level = 0; level < texData.numLevels(); level++) {
++        const auto imageSizeSlice = safeSlice(buf, offset, sizeof(quint32));
++        if (imageSizeSlice.isEmpty()) {
++            qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData());
++            return QTextureFileData();
++        }
++
++        const quint32 imageSize = decode(qFromUnaligned<quint32>(imageSizeSlice.data()));
++        offset += sizeof(quint32); // overflow checked indirectly above
++
++        texData.setDataOffset(offset, level);
++        texData.setDataLength(imageSize, level);
++
++        // Add image data and padding to offset
++        quint32 padded = 0;
++        if (nearestMultipleOf4(imageSize, &padded)) {
++            qWarning(lcQtGuiTextureIO, "Overflow in KTX file %s", logName().constData());
++            return QTextureFileData();
++        }
++
++        quint32 offsetNext;
++        if (qAddOverflow(offset, padded, &offsetNext)) {
++            qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData());
++            return QTextureFileData();
++        }
++
++        offset = offsetNext;
+     }
+
+     if (!texData.isValid()) {
+-        qCDebug(lcQtGuiTextureIO, "Invalid values in header of KTX file %s", logName().constData());
++        qWarning(lcQtGuiTextureIO, "Invalid values in header of KTX file %s",
++                 logName().constData());
+         return QTextureFileData();
+     }
+
+@@ -191,7 +271,7 @@ bool QKtxHandler::checkHeader(const KTXHeader &header)
+             (decode(header.numberOfFaces) == 1));
+ }
+
+-quint32 QKtxHandler::decode(quint32 val)
++quint32 QKtxHandler::decode(quint32 val) const
+ {
+     return inverseEndian ? qbswap<quint32>(val) : val;
+ }
+diff --git a/src/gui/util/qktxhandler_p.h b/src/gui/util/qktxhandler_p.h
+index f831e59d95..cdf1b2eaf8 100644
+--- a/src/gui/util/qktxhandler_p.h
++++ b/src/gui/util/qktxhandler_p.h
+@@ -68,7 +68,7 @@ public:
+
+ private:
+     bool checkHeader(const KTXHeader &header);
+-    quint32 decode(quint32 val);
++    quint32 decode(quint32 val) const;
+
+     bool inverseEndian = false;
+ };

qt5-qtbase.spec

@@ -57,7 +57,7 @@ BuildRequires: pkgconfig(libsystemd)
 Name:    qt5-qtbase
 Summary: Qt5 - QtBase components
 Version: 5.15.9
-Release: 7%{?dist}
+Release: 9%{?dist}
 
 
 # See LGPL_EXCEPTIONS.txt, for exception details
@@ -149,6 +149,9 @@ Patch112: CVE-2023-33285-qtbase-5.15.patch
 Patch113: CVE-2023-34410-qtbase-5.15.patch
 Patch114: CVE-2023-37369-qtbase-5.15.patch
 Patch115: CVE-2023-38197-qtbase-5.15.patch
+Patch116: 0001-CVE-2023-51714-qtbase-5.15.patch
+Patch117: 0002-CVE-2023-51714-qtbase-5.15.patch
+Patch118: CVE-2024-25580-qtbase-5.15.patch
 
 # gating related patches
 Patch200: qtbase-disable-tests-not-working-in-gating.patch
@@ -438,6 +441,9 @@ Qt5 libraries used for drawing widgets and OpenGL items.
 %patch -P113 -p1
 %patch -P114 -p1
 %patch -P115 -p1
+%patch -P116 -p1
+%patch -P117 -p1
+%patch -P118 -p1
 
 ## gating related patches
 %patch -P200 -p1 -b .disable-tests-not-working-in-gating
@@ -1138,6 +1144,14 @@ fi
 
 
 %changelog
+* Fri Feb 16 2024 Jan Grulich <jgrulich@redhat.com> - 5.15.9-9
+- Fix CVE-2024-25580: potential buffer overflow when reading KTX images
+  Resolves: RHEL-25726
+
+* Thu Jan 04 2024 Jan Grulich <jgrulich@redhat.com> - 5.15.9-8
+- Fix incorrect integer overflow check in HTTP2 implementation
+  Resolves: RHEL-20239
+
 * Fri Jul 21 2023 Jan Grulich <jgrulich@redhat.com> - 5.15.9-7
 - Fix infinite loops in QXmlStreamReader (CVE-2023-38197)
   Resolves: bz#2222771