[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1064056: marked as done (qtbase-opensource-src: CVE-2024-25580)

Your message dated Sun, 18 Feb 2024 00:37:49 +0000
with message-id <E1rbVBl-004jJz-0Z@fasolo.debian.org>
and subject line Bug#1064053: fixed in qtbase-opensource-src 5.15.10+dfsg-7
has caused the Debian Bug report #1064053,
regarding qtbase-opensource-src: CVE-2024-25580
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1064053: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1064053
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Source: qtbase-opensource-src
Version: 5.15.10+dfsg-6
Severity: normal
Tags: patch security

Dear Maintainer,

Security advisory CVE-2024-25580, a buffer overflow affecting KTX image
handling in QT, has been announced[1], and the announcement includes patches
for various versions of QT including the v5.15 branch.

I've confirmed that the patch applies cleanly to qtbase-opensource-src versions
5.15.8+dfsg-11 (bookworm / stable) and 5.15.10+dfsg-6 (trixie / testing), and
have successfully compiled the trixie package.

Please find attached the v5.15 patch from upstream.
( sha256sum 7cc9bf74f696de8ec5386bb80ce7a2fed5aa3870ac0e2c7db4628621c5c1a731 )

Regards,
James

[1] - https://lists.qt-project.org/pipermail/announce/2024-February/000472.html

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;
 };

--- End Message ---
--- Begin Message --- 
Source: qtbase-opensource-src
Source-Version: 5.15.10+dfsg-7
Done: Dmitry Shachnev <mitya57@debian.org>

We believe that the bug you reported is fixed in the latest version of
qtbase-opensource-src, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 1064053@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Dmitry Shachnev <mitya57@debian.org> (supplier of updated qtbase-opensource-src package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Sat, 17 Feb 2024 15:11:37 +0300
Source: qtbase-opensource-src
Architecture: source
Version: 5.15.10+dfsg-7
Distribution: unstable
Urgency: medium
Maintainer: Debian Qt/KDE Maintainers <debian-qt-kde@lists.debian.org>
Changed-By: Dmitry Shachnev <mitya57@debian.org>
Closes: 1064053
Changes:
 qtbase-opensource-src (5.15.10+dfsg-7) unstable; urgency=medium
 .
   * Backport upstream patch to fix potential buffer overflow when reading
     KTX images (CVE-2024-25580, closes: #1064053).
Checksums-Sha1:
 e7018036ef9626a5510d2dcc58043c5e896c4045 5312 qtbase-opensource-src_5.15.10+dfsg-7.dsc
 f84c35ee48ec3930bf7604e37446617cdb5cb0ae 237812 qtbase-opensource-src_5.15.10+dfsg-7.debian.tar.xz
 c99497b1cf1fb3fd4eedd02ccc4faf17f06da23c 16912 qtbase-opensource-src_5.15.10+dfsg-7_source.buildinfo
Checksums-Sha256:
 2641c71d71807422c60a025cf7fa1491e8bb021d45a40ca590b08925aa64d6e6 5312 qtbase-opensource-src_5.15.10+dfsg-7.dsc
 4a4f2afe86be116a08858eecfd5a419f0304547e22e6c8f75bea2e145f325a1c 237812 qtbase-opensource-src_5.15.10+dfsg-7.debian.tar.xz
 591825e004480f25d54e1814f347cf22e8572a14cbc53d277cf50919cb5989ce 16912 qtbase-opensource-src_5.15.10+dfsg-7_source.buildinfo
Files:
 792d5429e0fbfa25675c5e6b0520699b 5312 libs optional qtbase-opensource-src_5.15.10+dfsg-7.dsc
 50a8fa3ae71c217c6c66637508f6f138 237812 libs optional qtbase-opensource-src_5.15.10+dfsg-7.debian.tar.xz
 d3b953a84fa480a69bc35bc4eaa99fe1 16912 libs optional qtbase-opensource-src_5.15.10+dfsg-7_source.buildinfo

-----BEGIN PGP SIGNATURE-----

iQJHBAEBCgAxFiEEq2sdvrA0LydXHe1qsmYUtFL0RrYFAmXQo3wTHG1pdHlhNTdA
ZGViaWFuLm9yZwAKCRCyZhS0UvRGtrNCEACXPNqvTO5rLFSOroqSKn9+fK+xMNPj
8yV1wwkKXVCitjzJXVUO+GSx/lJE+nFxKhRL2DsRlm+RhGhvXp7tLwdozswCe4Uj
GO7I5sdpLCY3YGvLr3UKkKY+fg0o92yu43AUVZpRAFQsP2CB4pvGfIyVTxLCpk9c
+ZFI2SvFBJBSahj5sPiOABsxT7kLcK85mIMpoB8zM4Jy37xr+RBDqGnN8TnoO3lh
pYYNmi1dDIucadiKfDKWbnV9XMbFDLofyPm3P4o+zWOe3FlB4Xvtn0YsJdJddrTg
C3H7EVAPlsAIbhzltUQr3dtngaNAJBzBqL5uhiRGNFxr1zyXEghH0DO1lHuHfiSN
H3VOdpjqDw0Dd5DcGlJnmCKkWL3BQoMSFqKkCtus8KQA4S2ZJ7SvQh9RpZDWRu8b
WfNUkuDE4hsBXvV1KWyi5TkZjRJnkF6uDKw5dDjc7g+/vVXhgvjOlgN85zwuiC99
ejnzm5ZmwvcO9Xoe0zzwQU3vNv9TelAFlxFsHQFocV9T55CXWmsDW0PA6HM1c0kG
B/r+LV6bdCc0oJLU+0DJcpEYGoWVmDeQJxbF4/7MZ7LMLyzJ8EdvcYJ/tECn4jtF
Zj+2JhlENIEZ/0g//nS7Iav9GKI3qdi2M8ScNWIymFj9BpaELHMdKBt8cYszKkwY
GwQtlIS4pvQt7g==
=5rgS
-----END PGP SIGNATURE-----

Attachment: pgpHrEYtdIHCq.pgp
Description: PGP signature


--- End Message ---
Reply to: