From eb4da348a433c36e5091ff872d3849ebfd219df7 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 21 Aug 2026 18:22:26 +0200 Subject: [PATCH] QTextCodec: fix int overflow and infinite loop in ICU The ICU codec computes the required size of the encoded data using the following code: int requiredLength = UCNV_GET_MAX_BYTES_FOR_STRING(length, ucnv_getMaxCharSize(conv)); The macro expands into: (((int32_t)(length)+10)*(int32_t)(maxCharSize)) The maxCharSize parameter depends on the used encoding, and can be up to 8 chars per a code point - that is the case for ISO-2022-KR codec. Several other codecs also have maxCharSize == 6. As a result, for such codecs the requiredLength calculation can overflow even with relatively small input data. E.g., for maxCharSize == 8, it will be only around 250M input QChars, so roughly 500 Mb of text. Not even close to INT_MAX. The overflow itself is already UB, so should be fixed. In practice, all compilers these days simply provide a negative result. This negative requiredLength is then passed to the QByteArray constructor, which creates an empty QByteArray in response. As a result, the loop that tries to do the encoding keeps generating U_BUFFER_OVERFLOW_ERROR and tries to resize the bytearray with size = size * 2. But that’s obviously a no-op when the size is initially zero. So, we get an infinite loop. Fix it by expanding the macro and doing the initial requiredLength calculation in qint64. If the result is larger than INT_MAX, simply raise a warning and return an empty QByteArray immediately. We need to discard sizes larger than INT_MAX even in Qt 6, because ICU internally supports input and output buffers up to INT_MAX bytes. Obviously, for Qt 6 we could actually allocate the proper size, and then rewrite the encoding logic to do it in chunks. But considering that it's a compatibility module, I opted for a simpler approach, that can also be picked to 5.15 with minimal (if any) changes. The chunking can be implemented at the user side. The size * 2 calculation in the while() loop can overflow or exceed INT_MAX as well, so the patch applies the similar approach there. The same problem exists in the convertToUnicode() method: the output string unconditionally allocates length + 2 QChars, so the length cannot be larger than INT_MAX / sizeof(QChar) - 2, otherwise it'll cross the ICU buffer size limitation. Fix it similarly. [ChangeLog][QTextCodec][ICU] Fixed a bug in an ICU codec that could loop infinitely when the input to toUnicode() or fromUnicode() was too long. The updated code now raises a warning and returns an empty QString/QByteArray immediately. To process long inputs, implement the chunking of the input data on the client side, using the toUnicode() and fromUniocode() overloads that take the ConverterState as the last parameter. Conflict resolution for 6.8: * the Q_STATIC_LOGGING_CATEGORY macro does not exist in this branch. Use Q_LOGGING_CATEGORY instead. * removed the pick-to: 5.15, which needs to be done manually, anyway; the branch was registered for the Cherry-Pick Status dashboard, so it served its purpose. Fixes: QTBUG-149408 Pick-to: 6.5 Change-Id: Id523b9421b7cb364e48d6fcac2bbce98f968bd32 Reviewed-by: Thiago Macieira (cherry picked from commit afc1d04f0b6b5aaf2c1f1f070f824be4580efda4) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 0768c828f42da4bff5ea06bef2f6bfda0aa1cb5c) (cherry picked from commit 2e3b94b61d84aa93a6837a1fdfbc39c26756850b) Reviewed-by: Marc Mutz --- diff --git a/src/core5/codecs/qicucodec.cpp b/src/core5/codecs/qicucodec.cpp index a2cf0e0..9544891 100644 --- a/src/core5/codecs/qicucodec.cpp +++ b/src/core5/codecs/qicucodec.cpp @@ -9,6 +9,7 @@ #include "qlatincodec_p.h" #include "qsimplecodec_p.h" #include "qdebug.h" +#include #include "unicode/ucnv.h" @@ -19,9 +20,18 @@ QT_BEGIN_NAMESPACE +Q_LOGGING_CATEGORY(lcIcu, "qt.core5compat.qtextcodec.icu"); + typedef QList::ConstIterator TextCodecListConstIt; typedef QList::ConstIterator ByteArrayListConstIt; +Q_DECL_COLD_FUNCTION +static void warnOnLongInput() +{ + qWarning(lcIcu, "Unable to allocate an output buffer of a proper size - the input" + " is too long. Pass the input data in smaller chunks instead."); +} + static void qIcuCodecStateFree(QTextCodec::ConverterState *state) noexcept { ucnv_close(static_cast(state->d[0])); @@ -596,7 +606,16 @@ { UConverter *conv = getConverter(state); - QString string(length + 2, QT_PREPEND_NAMESPACE(Qt::Uninitialized)); + // ICU requires the input and output buffers to stay within INT_MAX bytes. + constexpr int MaxStringSize = (std::numeric_limits::max)() / sizeof(QChar); + + QString string; + if (length > MaxStringSize - 2) { + warnOnLongInput(); + return string; + } + + string.resize(length + 2); const char *end = chars + length; int convertedChars = 0; @@ -620,7 +639,11 @@ convertedChars = uc - (UChar *)string.data(); if (chars >= end) break; - string.resize(string.size()*2); + // Increase the size, but keep in mind that ICU is limited by INT_MAX bytes + if (string.size() <= MaxStringSize / 2) + string.resize(string.size() * 2); + else + break; } string.resize(convertedChars); @@ -634,8 +657,20 @@ { UConverter *conv = getConverter(state); - int requiredLength = UCNV_GET_MAX_BYTES_FOR_STRING(length, ucnv_getMaxCharSize(conv)); - QByteArray string(requiredLength, QT_PREPEND_NAMESPACE(Qt::Uninitialized)); + QByteArray string; + + // That was UCNV_GET_MAX_BYTES_FOR_STRING, but we want to do the calculation in + // int64, so that the result does not overflow + qint64 requiredLength = (qint64{length} + 10) * qint64{ucnv_getMaxCharSize(conv)}; + + // ICU requires the input and output buffers to stay within INT_MAX bytes. + constexpr int MaxBufferSize = (std::numeric_limits::max)(); + if (requiredLength > MaxBufferSize) { + warnOnLongInput(); + return string; + } + + string.resize(static_cast(requiredLength)); const UChar *uc = (const UChar *)unicode; const UChar *end = uc + length; @@ -658,7 +693,12 @@ convertedChars = ch - string.data(); if (uc >= end) break; - string.resize(string.size()*2); + // Increase the size, but keep in mind that the calculation can overflow, + // and that ICU is limited by INT_MAX + if (string.size() <= MaxBufferSize / 2) + string.resize(string.size() * 2); + else + break; } string.resize(convertedChars); diff --git a/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp b/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp index d3181a0..2bf924b 100644 --- a/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp +++ b/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp @@ -97,6 +97,8 @@ void tsciiEncoderDoesNotReadPastTheEnd(); void iconvEncoderPendingSurrogateDoesNotOverflow(); + + void icuNoIntOverflowInFromUnicode(); }; void tst_QTextCodec::toUnicode_data() @@ -2931,6 +2933,35 @@ QCOMPARE(state.remainingChars, qsizetype(0)); } +/* + The ICU codec uses UCNV_GET_MAX_BYTES_FOR_STRING() macro that can + overflow on a long input string. As a result, we'll pass a negative + length to the c-tor of QByteArray, which will effectively lead to using + a global static empty instance. The code that then tries to populate + this buffer with the actual encoding results, will get stuck in an infinite + loop without a fix. +*/ +void tst_QTextCodec::icuNoIntOverflowInFromUnicode() +{ + // Only ICU supports this codec + QTextCodec *codec = QTextCodec::codecForName("ISO-2022-KR"); // maxCharSize == 8 + if (!codec) + QSKIP("This test requires ICU"); + + QT_TRY { + // Enough to trigger int overflow when calculating max buffer size + constexpr qsizetype InputLength = 300 * 1024 * 1024; + const QString input(InputLength, u'x'); + + QTest::ignoreMessage(QtWarningMsg, + QRegularExpression("Unable to allocate an output buffer.+")); + const QByteArray result = codec->fromUnicode(input); + QVERIFY(result.isNull()); + } QT_CATCH(const std::bad_alloc &) { + QSKIP("The test could not allocate enough memory"); + } +} + QT_END_NAMESPACE QTEST_MAIN(tst_QTextCodec)