From eeec0c48f5f108f380175e9c9a1a8bb77bb7718c Mon Sep 17 00:00:00 2001 From: zhaosai Date: Thu, 9 May 2024 15:49:53 +0800 Subject: [PATCH] fix CVE-2024-25580 --- CVE-2024-25580.patch | 407 +++++++++++++++++++++++++++++++++++++++++++ qt.spec | 9 +- 2 files changed, 415 insertions(+), 1 deletion(-) create mode 100644 CVE-2024-25580.patch diff --git a/CVE-2024-25580.patch b/CVE-2024-25580.patch new file mode 100644 index 0000000..fdfd50b --- /dev/null +++ b/CVE-2024-25580.patch @@ -0,0 +1,407 @@ +From dec1863c7dc63e5788b0c6c061d36e856a6ae2b2 Mon Sep 17 00:00:00 2001 +From: Jonas Karlsson +Date: Thu, 8 Feb 2024 17:01:05 +0100 +Subject: Improve KTX file reading memory safety + +* Use qAddOverflow/qSubOverflow methods for catching additions and + subtractions with overflow and handle these scenarios when reading the + file. +* Add 'safeView' method that checks that the byte array view constructed + is not out of bounds. +* Return error if number of levels is higher than what is reasonable. +* Return error if number of faces is incorrect. +* Add unit test with invalid KTX file previously causing a segmentation + fault. + +This fixes CVE-2024-25580. + +Fixes: QTBUG-121918 +Change-Id: Ie0824c32a5921de30cf07c1fc1b49a084e6d07b2 +Reviewed-by: Eirik Aavitsland +Reviewed-by: Qt CI Bot +(cherry picked from commit 28ecb523ce8490bff38b251b3df703c72e057519) +Reviewed-by: Jonas Karlsson +--- + src/gui/util/qktxhandler.cpp | 231 ++++++++++++++++----- + src/gui/util/qktxhandler_p.h | 4 +- + .../gui/util/qtexturefilereader/CMakeLists.txt | 1 + + .../qtexturefilereader/texturefiles/invalid.ktx | Bin 0 -> 69 bytes + .../qtexturefilereader/tst_qtexturefilereader.cpp | 13 ++ + 5 files changed, 197 insertions(+), 52 deletions(-) + create mode 100644 tests/auto/gui/util/qtexturefilereader/texturefiles/invalid.ktx + +diff --git a/src/gui/util/qktxhandler.cpp b/src/gui/util/qktxhandler.cpp +index f04da929c3..d6948cc01d 100644 +--- a/src/gui/util/qktxhandler.cpp ++++ b/src/gui/util/qktxhandler.cpp +@@ -41,7 +41,7 @@ struct KTXHeader { + quint32 bytesOfKeyValueData; + }; + +-static const quint32 qktxh_headerSize = sizeof(KTXHeader); ++static constexpr quint32 qktxh_headerSize = sizeof(KTXHeader); + + // Currently unused, declared for future reference + struct KTXKeyValuePairItem { +@@ -71,11 +71,24 @@ struct KTXMipmapLevel { + */ + }; + +-// Returns the nearest multiple of 'rounding' greater than or equal to 'value' +-constexpr quint32 withPadding(quint32 value, quint32 rounding) ++// Returns the nearest multiple of 4 greater than or equal to 'value' ++static const std::optional nearestMultipleOf4(quint32 value) + { +- Q_ASSERT(rounding > 1); +- return value + (rounding - 1) - ((value + (rounding - 1)) % rounding); ++ constexpr quint32 rounding = 4; ++ quint32 result = 0; ++ if (qAddOverflow(value, rounding - 1, &result)) ++ return std::nullopt; ++ result &= ~(rounding - 1); ++ return result; ++} ++ ++// Returns a view with prechecked bounds ++static QByteArrayView safeView(QByteArrayView view, quint32 start, quint32 length) ++{ ++ quint32 end = 0; ++ if (qAddOverflow(start, length, &end) || end > quint32(view.length())) ++ return {}; ++ return view.sliced(start, length); + } + + QKtxHandler::~QKtxHandler() = default; +@@ -83,8 +96,7 @@ QKtxHandler::~QKtxHandler() = default; + bool QKtxHandler::canRead(const QByteArray &suffix, const QByteArray &block) + { + Q_UNUSED(suffix); +- +- return (qstrncmp(block.constData(), ktxIdentifier, KTX_IDENTIFIER_LENGTH) == 0); ++ return block.startsWith(ktxIdentifier); + } + + QTextureFileData QKtxHandler::read() +@@ -93,55 +105,122 @@ QTextureFileData QKtxHandler::read() + return QTextureFileData(); + + const QByteArray buf = device()->readAll(); +- const quint32 dataSize = quint32(buf.size()); +- if (dataSize < qktxh_headerSize || !canRead(QByteArray(), buf)) { +- qCDebug(lcQtGuiTextureIO, "Invalid KTX file %s", logName().constData()); ++ if (buf.size() > std::numeric_limits::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(); + } + +- const KTXHeader *header = reinterpret_cast(buf.data()); +- if (!checkHeader(*header)) { +- qCDebug(lcQtGuiTextureIO, "Unsupported KTX file format in %s", logName().constData()); ++ if (buf.size() < qsizetype(qktxh_headerSize)) { ++ qWarning(lcQtGuiTextureIO, "Invalid KTX header size in %s", logName().constData()); ++ return QTextureFileData(); ++ } ++ ++ 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.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)); +- texData.setNumFaces(decode(header->numberOfFaces)); ++ texData.setNumLevels(decode(header.numberOfMipmapLevels)); ++ texData.setNumFaces(decode(header.numberOfFaces)); ++ ++ 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(); ++ } + +- const quint32 bytesOfKeyValueData = decode(header->bytesOfKeyValueData); +- if (qktxh_headerSize + bytesOfKeyValueData < quint64(buf.size())) // oob check +- texData.setKeyValueMetadata(decodeKeyValues( +- QByteArrayView(buf.data() + qktxh_headerSize, bytesOfKeyValueData))); +- quint32 offset = qktxh_headerSize + bytesOfKeyValueData; ++ if (headerKeyValueSize >= quint32(buf.size())) { ++ qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData()); ++ return QTextureFileData(); ++ } ++ ++ // File contains key/values ++ if (bytesOfKeyValueData > 0) { ++ auto keyValueDataView = safeView(buf, qktxh_headerSize, bytesOfKeyValueData); ++ if (keyValueDataView.isEmpty()) { ++ qWarning(lcQtGuiTextureIO, "Invalid view in KTX file %s", logName().constData()); ++ return QTextureFileData(); ++ } ++ ++ auto keyValues = decodeKeyValues(keyValueDataView); ++ if (!keyValues) { ++ qWarning(lcQtGuiTextureIO, "Could not parse key values in KTX file %s", ++ logName().constData()); ++ return QTextureFileData(); ++ } ++ ++ texData.setKeyValueMetadata(*keyValues); ++ } ++ ++ // 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(); ++ } + +- constexpr int MAX_ITERATIONS = 32; // cap iterations in case of corrupt data ++ if (texData.numFaces() != 1 && texData.numFaces() != 6) { ++ qWarning(lcQtGuiTextureIO, "Invalid number of faces in KTX file %s", logName().constData()); ++ return QTextureFileData(); ++ } + +- for (int level = 0; level < qMin(texData.numLevels(), MAX_ITERATIONS); level++) { +- if (offset + sizeof(quint32) > dataSize) // Corrupt file; avoid oob read +- break; ++ quint32 offset = headerKeyValueSize; ++ for (int level = 0; level < texData.numLevels(); level++) { ++ const auto imageSizeView = safeView(buf, offset, sizeof(quint32)); ++ if (imageSizeView.isEmpty()) { ++ qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData()); ++ return QTextureFileData(); ++ } + +- const quint32 imageSize = decode(qFromUnaligned(buf.data() + offset)); +- offset += sizeof(quint32); ++ const quint32 imageSize = decode(qFromUnaligned(imageSizeView.data())); ++ offset += sizeof(quint32); // overflow checked indirectly above + +- for (int face = 0; face < qMin(texData.numFaces(), MAX_ITERATIONS); face++) { ++ for (int face = 0; face < texData.numFaces(); face++) { + texData.setDataOffset(offset, level, face); + texData.setDataLength(imageSize, level, face); + + // Add image data and padding to offset +- offset += withPadding(imageSize, 4); ++ const auto padded = nearestMultipleOf4(imageSize); ++ if (!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(); + } + +@@ -187,33 +266,83 @@ bool QKtxHandler::checkHeader(const KTXHeader &header) + return is2D && (isCubeMap || isCompressedImage); + } + +-QMap QKtxHandler::decodeKeyValues(QByteArrayView view) const ++std::optional> QKtxHandler::decodeKeyValues(QByteArrayView view) const + { + QMap output; + quint32 offset = 0; +- while (offset < view.size() + sizeof(quint32)) { ++ while (offset < quint32(view.size())) { ++ const auto keyAndValueByteSizeView = safeView(view, offset, sizeof(quint32)); ++ if (keyAndValueByteSizeView.isEmpty()) { ++ qWarning(lcQtGuiTextureIO, "Invalid view in KTX key-value"); ++ return std::nullopt; ++ } ++ + const quint32 keyAndValueByteSize = +- decode(qFromUnaligned(view.constData() + offset)); +- offset += sizeof(quint32); ++ decode(qFromUnaligned(keyAndValueByteSizeView.data())); + +- if (offset + keyAndValueByteSize > quint64(view.size())) +- break; // oob read ++ quint32 offsetKeyAndValueStart; ++ if (qAddOverflow(offset, quint32(sizeof(quint32)), &offsetKeyAndValueStart)) { ++ qWarning(lcQtGuiTextureIO, "Overflow in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ quint32 offsetKeyAndValueEnd; ++ if (qAddOverflow(offsetKeyAndValueStart, keyAndValueByteSize, &offsetKeyAndValueEnd)) { ++ qWarning(lcQtGuiTextureIO, "Overflow in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ const auto keyValueView = safeView(view, offsetKeyAndValueStart, keyAndValueByteSize); ++ if (keyValueView.isEmpty()) { ++ qWarning(lcQtGuiTextureIO, "Invalid view in KTX key-value"); ++ return std::nullopt; ++ } + + // 'key' is a UTF-8 string ending with a null terminator, 'value' is the rest. + // To separate the key and value we convert the complete data to utf-8 and find the first + // null terminator from the left, here we split the data into two. +- const auto str = QString::fromUtf8(view.constData() + offset, keyAndValueByteSize); +- const int idx = str.indexOf('\0'_L1); +- if (idx == -1) +- continue; +- +- const QByteArray key = str.left(idx).toUtf8(); +- const size_t keySize = key.size() + 1; // Actual data size +- const QByteArray value = QByteArray::fromRawData(view.constData() + offset + keySize, +- keyAndValueByteSize - keySize); +- +- offset = withPadding(offset + keyAndValueByteSize, 4); +- output.insert(key, value); ++ ++ const int idx = keyValueView.indexOf('\0'); ++ if (idx == -1) { ++ qWarning(lcQtGuiTextureIO, "Invalid key in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ const QByteArrayView keyView = safeView(view, offsetKeyAndValueStart, idx); ++ if (keyView.isEmpty()) { ++ qWarning(lcQtGuiTextureIO, "Overflow in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ const quint32 keySize = idx + 1; // Actual data size ++ ++ quint32 offsetValueStart; ++ if (qAddOverflow(offsetKeyAndValueStart, keySize, &offsetValueStart)) { ++ qWarning(lcQtGuiTextureIO, "Overflow in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ quint32 valueSize; ++ if (qSubOverflow(keyAndValueByteSize, keySize, &valueSize)) { ++ qWarning(lcQtGuiTextureIO, "Underflow in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ const QByteArrayView valueView = safeView(view, offsetValueStart, valueSize); ++ if (valueView.isEmpty()) { ++ qWarning(lcQtGuiTextureIO, "Invalid view in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ output.insert(keyView.toByteArray(), valueView.toByteArray()); ++ ++ const auto offsetNext = nearestMultipleOf4(offsetKeyAndValueEnd); ++ if (!offsetNext) { ++ qWarning(lcQtGuiTextureIO, "Overflow in KTX key-value"); ++ return std::nullopt; ++ } ++ ++ offset = *offsetNext; + } + + return output; +diff --git a/src/gui/util/qktxhandler_p.h b/src/gui/util/qktxhandler_p.h +index 7d54b20922..3a0b8fcf7e 100644 +--- a/src/gui/util/qktxhandler_p.h ++++ b/src/gui/util/qktxhandler_p.h +@@ -17,6 +17,8 @@ + + #include "qtexturefilehandler_p.h" + ++#include ++ + QT_BEGIN_NAMESPACE + + struct KTXHeader; +@@ -33,7 +35,7 @@ public: + + private: + bool checkHeader(const KTXHeader &header); +- QMap decodeKeyValues(QByteArrayView view) const; ++ std::optional> decodeKeyValues(QByteArrayView view) const; + quint32 decode(quint32 val) const; + + bool inverseEndian = false; +diff --git a/tests/auto/gui/util/qtexturefilereader/CMakeLists.txt b/tests/auto/gui/util/qtexturefilereader/CMakeLists.txt +index 2949dfd0d6..7efdb2c23d 100644 +--- a/tests/auto/gui/util/qtexturefilereader/CMakeLists.txt ++++ b/tests/auto/gui/util/qtexturefilereader/CMakeLists.txt +@@ -11,6 +11,7 @@ set(qtexturefilereader_resource_files + "texturefiles/car_mips.ktx" + "texturefiles/cubemap_float32_rgba.ktx" + "texturefiles/cubemap_metadata.ktx" ++ "texturefiles/invalid.ktx" + "texturefiles/newlogo.astc" + "texturefiles/newlogo_srgb.astc" + "texturefiles/pattern.pkm" +diff --git a/tests/auto/gui/util/qtexturefilereader/texturefiles/invalid.ktx b/tests/auto/gui/util/qtexturefilereader/texturefiles/invalid.ktx +new file mode 100644 +index 0000000000..68a92221db +Binary files /dev/null and b/tests/auto/gui/util/qtexturefilereader/texturefiles/invalid.ktx differ +diff --git a/tests/auto/gui/util/qtexturefilereader/tst_qtexturefilereader.cpp b/tests/auto/gui/util/qtexturefilereader/tst_qtexturefilereader.cpp +index 9d7205a921..41d0acfab2 100644 +--- a/tests/auto/gui/util/qtexturefilereader/tst_qtexturefilereader.cpp ++++ b/tests/auto/gui/util/qtexturefilereader/tst_qtexturefilereader.cpp +@@ -11,6 +11,7 @@ class tst_qtexturefilereader : public QObject + private slots: + void checkHandlers_data(); + void checkHandlers(); ++ void checkInvalid(); + void checkMetadata(); + }; + +@@ -140,6 +141,18 @@ void tst_qtexturefilereader::checkMetadata() + QCOMPARE(kvs.value("test C"), QByteArrayLiteral("3\x0000")); + } + ++void tst_qtexturefilereader::checkInvalid() ++{ ++ QFile f(":/texturefiles/invalid.ktx"); ++ QVERIFY(f.open(QIODevice::ReadOnly)); ++ QTextureFileReader r(&f); ++ QTextureFileData d = r.read(); ++ auto kvs = d.keyValueMetadata(); ++ ++ // Basically just checking that we don't crash on and invalid file ++ QVERIFY(kvs.empty()); ++} ++ + QTEST_MAIN(tst_qtexturefilereader) + + #include "tst_qtexturefilereader.moc" +-- +cgit v1.2.3 + diff --git a/qt.spec b/qt.spec index 36a4654..3b9fffe 100644 --- a/qt.spec +++ b/qt.spec @@ -13,7 +13,7 @@ Name: qt Epoch: 1 Version: 4.8.7 -Release: 60 +Release: 61 Summary: A software toolkit for developing applications License: (LGPLv2 with exceptions or GPLv3 with exceptions) and ASL 2.0 and BSD and FTL and MIT URL: http://qt-project.org/ @@ -93,6 +93,7 @@ Patch6008: qt-CVE-2023-34410.patch Patch6009: qt-CVE-2023-38197.patch Patch6010: qt-CVE-2023-37369.patch Patch6011: qt-CVE-2023-43114.patch +Patch6012: CVE-2024-25580.patch BuildRequires: cups-devel desktop-file-utils gcc-c++ libjpeg-devel findutils libmng-devel libtiff-devel pkgconfig pkgconfig(alsa) BuildRequires: pkgconfig(dbus-1) pkgconfig(fontconfig) pkgconfig(glib-2.0) pkgconfig(icu-i18n) openssl-devel pkgconfig(libpng) @@ -470,6 +471,12 @@ fi %{_qt4_prefix}/examples/ %changelog +* Thu May 09 2024 zhaosai - 1:4.8.7-61 +- Type:cves +- ID:CVE-2024-25580 +- SUG:NA +- DESC:fix CVE-2024-25580 + * Mon Nov 27 2023 hua_yadong - 1:4.8.7-60 - Type:cves - ID:CVE-2023-43114 -- Gitee