From fff3cbfc0aa1ff205c489b6241d96ae64e4b7892 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 21 Aug 2026 15:53:08 +0200 Subject: [PATCH] QTextCodec: fix OOB read in TSCII codec The TSCII codec iterates over the input buffer, and if the currently processed QChar is not ASCII (has a value larger than 0x80), tries to analyze the next two QChars. The problem is that it does so without validating the i + 1 and i + 2 indices against the input length. As a result, if it can try to read up to two QChars past the end of the input buffer. The test is crafted in such way, that a larger buffer is passed as an input, so it does not trigger ASAN in case of failure, but rather simply reports an invalid result. Note that the else branch in the code that now records an error could instead save the trailing QChar in the converter state, so that the following chunk could decode it properly. But this patch does not try to fix the correctness of the encoding, it simply eliminates the read past the end of the input buffer. Disclaimer: the tests were developed with the help of AI. Task-number: QTBUG-149214 Pick-to: 6.5 5.15 Change-Id: Ibbb7cbdae7ddd4cba52bf69002b26dd076d68372 Reviewed-by: Thiago Macieira (cherry picked from commit 5841a625142199bf3c58e6ace6fa2c4a8a2c5254) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 9b5b9a57e72c9d69197b99ab4a3939833d334377) (cherry picked from commit 7b8ed3e3f8dad3c2fe76ebe68b121010c3ca19b6) --- diff --git a/src/core5/codecs/qtsciicodec.cpp b/src/core5/codecs/qtsciicodec.cpp index c5151da..cb492b2 100644 --- a/src/core5/codecs/qtsciicodec.cpp +++ b/src/core5/codecs/qtsciicodec.cpp @@ -50,13 +50,13 @@ if (ch.row() == 0x00 && ch.cell() < 0x80) { // ASCII j = ch.cell(); - } else if ((j = qt_UnicodeToTSCII(uc[i].unicode(), - uc[i + 1].unicode(), - uc[i + 2].unicode()))) { + } else if ((i < len - 2) && (j = qt_UnicodeToTSCII(uc[i].unicode(), + uc[i + 1].unicode(), + uc[i + 2].unicode()))) { // We have to check the combined chars first! i += 2; - } else if ((j = qt_UnicodeToTSCII(uc[i].unicode(), - uc[i + 1].unicode(), 0))) { + } else if ((i < len - 1) && (j = qt_UnicodeToTSCII(uc[i].unicode(), + uc[i + 1].unicode(), 0))) { i++; } else if ((j = qt_UnicodeToTSCII(uc[i].unicode(), 0, 0))) { } else { diff --git a/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp b/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp index b898a92..f00bdba 100644 --- a/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp +++ b/tests/auto/core5/codecs/qtextcodec/tst_qtextcodec.cpp @@ -92,6 +92,9 @@ void canEncode(); void canEncode_data(); + + void tsciiEncoderDoesNotReadPastTheEnd_data(); + void tsciiEncoderDoesNotReadPastTheEnd(); }; void tst_QTextCodec::toUnicode_data() @@ -2816,6 +2819,75 @@ QTest::newRow("Pencil icon") << "ISO-8859-1" << QString("\u270f") << QByteArray("?") << false; } +void tst_QTextCodec::tsciiEncoderDoesNotReadPastTheEnd_data() +{ + QTest::addColumn("input"); + QTest::addColumn("beyond"); + QTest::addColumn("expected"); + + // A single non-ASCII character: both uc[len] and uc[len + 1] are read, + // and U+0BE7 U+0BB7 U+0B82 is the one three-character sequence in the + // table, so the encoder answers 0x8C instead of 0x81 for U+0BE7 alone. + QTest::newRow("last-char") + << QStringLiteral("\u0BE7") + << QStringLiteral("\u0BB7\u0B82") + << QByteArray("\x81"); + + // Non-ASCII in the second to last position: only uc[len] is read, and + // it turns the two-character U+0BE7 U+0BB7 (0x87) into 0x8C again. + QTest::newRow("second-to-last-char") + << QStringLiteral("\u0BE7\u0BB7") + << QStringLiteral("\u0B82\u0000") + << QByteArray("\x87"); + + // Valid cases. + QTest::newRow("valid-three-chars") + << QStringLiteral("\u0BE7\u0BB7\u0B82") + << QStringLiteral("\u0BC1\u0000") + << QByteArray("\x8C"); + QTest::newRow("valid-two-chars") + << QStringLiteral("\u0B95\u0BC2") + << QStringLiteral("\u0BCD\u0000") + << QByteArray("\xDC"); +} + +/* + This test hands the codec a buffer that is longer than the length it is + told about, and checks that the conversion of the first input.size() + characters does not depend on what follows them. + + Before the fix, the look-ahead in QTsciiCodec::convertFromUnicode() + accesses uc[i + 1] and uc[i + 2] without checking the length, so this + fails with the wrong bytes. + In practice that could trigger a read past the end of the buffer. +*/ +void tst_QTextCodec::tsciiEncoderDoesNotReadPastTheEnd() +{ + QFETCH(const QString, input); + QFETCH(const QString, beyond); + QFETCH(const QByteArray, expected); + + QTextCodec *codec = QTextCodec::codecForName("TSCII"); + QVERIFY(codec); + + QCOMPARE(beyond.size(), 2); + // One buffer, holding the input followed by the characters that stand for + // whatever happens to be in memory after it. Only input.size() characters + // are ever passed to the codec. + const QString buffer = input + beyond; + + const QChar *data = buffer.constData(); + const int len = int(input.size()); + + QTextCodec::ConverterState state; + const QByteArray result = codec->fromUnicode(data, len, &state); + QCOMPARE(result, expected); + + // Test with Stateless flag + const QByteArray stateless = codec->fromUnicode(QStringView(data, input.size())); + QCOMPARE(stateless, expected); +} + QT_END_NAMESPACE QTEST_MAIN(tst_QTextCodec)