Skip to content

Commit

Permalink
[qtbase] add official patch for CVE-2024-25580 (#36822)
Browse files Browse the repository at this point in the history 
Fixes #36821.

- [x] Changes comply with the [maintainer
guide](https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md).
- [x] SHA512s are updated for each updated download.
- [x] The "supports" clause reflects platforms that may be fixed by this
new version.
- [x] Any fixed [CI
baseline](https://github.com/microsoft/vcpkg/blob/master/scripts/ci.baseline.txt)
entries are removed from that file.
- [x] Any patches that are no longer applied are deleted from the port's
directory.
- [x] The version database is fixed by rerunning `./vcpkg x-add-version
--all` and committing the result.
- [x] Only one version is added to each modified port's versions file.
  • Loading branch information
@carsten-grimm
carsten-grimm authored Feb 21, 2024
1 parent 62f4702 commit 4bee3f5
Show file tree
Hide file tree
Showing 5 changed files with 333 additions and 2 deletions.
325 changes: 325 additions & 0 deletions ports/qtbase/patches/CVE-2024-25580-qtbase-6.6.diff
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,325 @@
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<quint32> 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<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();
}

- const KTXHeader *header = reinterpret_cast<const KTXHeader *>(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<quint32>(buf.data() + offset));
- offset += sizeof(quint32);
+ const quint32 imageSize = decode(qFromUnaligned<quint32>(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<QByteArray, QByteArray> QKtxHandler::decodeKeyValues(QByteArrayView view) const
+std::optional<QMap<QByteArray, QByteArray>> QKtxHandler::decodeKeyValues(QByteArrayView view) const
{
QMap<QByteArray, QByteArray> 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<quint32>(view.constData() + offset));
- offset += sizeof(quint32);
+ decode(qFromUnaligned<quint32>(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 <optional>
+
QT_BEGIN_NAMESPACE

struct KTXHeader;
@@ -33,7 +35,7 @@ public:

private:
bool checkHeader(const KTXHeader &header);
- QMap<QByteArray, QByteArray> decodeKeyValues(QByteArrayView view) const;
+ std::optional<QMap<QByteArray, QByteArray>> decodeKeyValues(QByteArrayView view) const;
quint32 decode(quint32 val) const;

bool inverseEndian = false;
1 change: 1 addition & 0 deletions ports/qtbase/portfile.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ set(${PORT}_PATCHES
# CVE fixes from https://download.qt.io/official_releases/qt/6.6/
patches/0001-CVE-2023-51714-qtbase-6.6.diff
patches/0002-CVE-2023-51714-qtbase-6.6.diff
patches/CVE-2024-25580-qtbase-6.6.diff

allow_outside_prefix.patch
config_install.patch
Expand Down
2 changes: 1 addition & 1 deletion ports/qtbase/vcpkg.json
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "qtbase",
"version": "6.6.1",
"port-version": 9,
"port-version": 10,
"description": "Qt Application Framework Base Module. Includes Core, GUI, Widgets, Networking, SQL, Concurrent and other essential qt components.",
"homepage": "https://www.qt.io/",
"license": null,
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7198,7 +7198,7 @@
},
"qtbase": {
"baseline": "6.6.1",
"port-version": 9
"port-version": 10
},
"qtcharts": {
"baseline": "6.6.1",
Expand Down
5 changes: 5 additions & 0 deletions versions/q-/qtbase.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "697f89ff528ad2e1e71821f9c549855dd62a2531",
"version": "6.6.1",
"port-version": 10
},
{
"git-tree": "99b63c90d57e03fb23c3750b28f70e810c981474",
"version": "6.6.1",
Expand Down

0 comments on commit 4bee3f5

Please sign in to comment.