147 lines
5.9 KiB
Diff
147 lines
5.9 KiB
Diff
From 7b8ed3e3f8dad3c2fe76ebe68b121010c3ca19b6 Mon Sep 17 00:00:00 2001
|
|
From: Ivan Solovev <ivan.solovev@qt.io>
|
|
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.8 6.5 5.15
|
|
Change-Id: Ibbb7cbdae7ddd4cba52bf69002b26dd076d68372
|
|
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
|
|
(cherry picked from commit 5841a625142199bf3c58e6ace6fa2c4a8a2c5254)
|
|
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
|
|
(cherry picked from commit 9b5b9a57e72c9d69197b99ab4a3939833d334377)
|
|
---
|
|
|
|
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 086cf07..dcb5390 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<QString>("input");
|
|
+ QTest::addColumn<QString>("beyond");
|
|
+ QTest::addColumn<QByteArray>("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)
|