QCborValue: catch overflow in QByteArray when decoding chunked strings

We checked against integer overflow, but not against overflowing the
QByteArray size limit. That caused a std::bad_alloc to be thrown, which
is bad when decoding unknown data. QCborStreamReader wasn't affected,
since it doesn't merge chunks.

Change-Id: I99ab0f318b1c43b89888fffd160c36f495fada87
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Thiago Macieira 2020-05-05 11:59:52 -07:00
parent 66908badac
commit 798492ccee
3 changed files with 31 additions and 6 deletions

View File

@ -1636,7 +1636,7 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader)
if (len == rawlen) { if (len == rawlen) {
auto oldSize = data.size(); auto oldSize = data.size();
auto newSize = oldSize; auto newSize = oldSize;
if (!add_overflow(newSize, len, &newSize)) { if (!add_overflow(newSize, len, &newSize) && newSize < MaxByteArraySize) {
if (newSize != oldSize) if (newSize != oldSize)
data.resize(newSize); data.resize(newSize);

View File

@ -81,19 +81,31 @@ qint64 LargeIODevice::readData(char *data, qint64 maxlen)
void addValidationLargeData(qsizetype minInvalid, qsizetype maxInvalid) void addValidationLargeData(qsizetype minInvalid, qsizetype maxInvalid)
{ {
char toolong[2 + sizeof(qsizetype)] = { char(0x81) }; char toolong[1 + sizeof(qsizetype)];
for (qsizetype v = maxInvalid; v >= minInvalid; --v) { for (qsizetype v = maxInvalid; v >= minInvalid; --v) {
// 0x5a for 32-bit, 0x5b for 64-bit // 0x5a for 32-bit, 0x5b for 64-bit
toolong[1] = sizeof(v) > 4 ? 0x5b : 0x5a; toolong[0] = sizeof(v) > 4 ? 0x5b : 0x5a;
qToBigEndian(v, toolong + 2); qToBigEndian(v, toolong + 1);
QTest::addRow("bytearray-too-big-for-qbytearray-%llx", v) QTest::addRow("bytearray-too-big-for-qbytearray-%llx", v)
<< QByteArray(toolong, sizeof(toolong)) << 0 << CborErrorDataTooLarge; << QByteArray(toolong, sizeof(toolong)) << 0 << CborErrorDataTooLarge;
toolong[1] |= 0x20; QTest::addRow("bytearray-chunked-too-big-for-qbytearray-%llx", v)
<< ('\x5f' + QByteArray(toolong, sizeof(toolong)) + '\xff')
<< 0 << CborErrorDataTooLarge;
QTest::addRow("bytearray-2chunked-too-big-for-qbytearray-%llx", v)
<< ("\x5f\x40" + QByteArray(toolong, sizeof(toolong)) + '\xff')
<< 0 << CborErrorDataTooLarge;
toolong[0] |= 0x20;
// QCborStreamReader::readString copies to a QByteArray first // QCborStreamReader::readString copies to a QByteArray first
QTest::addRow("string-too-big-for-qbytearray-%llx", v) QTest::addRow("string-too-big-for-qbytearray-%llx", v)
<< QByteArray(toolong, sizeof(toolong)) << 0 << CborErrorDataTooLarge; << QByteArray(toolong, sizeof(toolong)) << 0 << CborErrorDataTooLarge;
QTest::addRow("string-chunked-too-big-for-qbytearray-%llx", v)
<< ('\x7f' + QByteArray(toolong, sizeof(toolong)) + '\xff')
<< 0 << CborErrorDataTooLarge;
QTest::addRow("string-2chunked-too-big-for-qbytearray-%llx", v)
<< ("\x7f\x60" + QByteArray(toolong, sizeof(toolong)) + '\xff')
<< 0 << CborErrorDataTooLarge;
} }
} }

View File

@ -1926,11 +1926,24 @@ void tst_QCborValue::validation_data()
// Add QCborStreamReader-specific limitations due to use of QByteArray and // Add QCborStreamReader-specific limitations due to use of QByteArray and
// QString, which are allocated by QArrayData::allocate(). // QString, which are allocated by QArrayData::allocate().
const qsizetype MaxInvalid = std::numeric_limits<QByteArray::size_type>::max(); const qsizetype MaxInvalid = std::numeric_limits<QByteArray::size_type>::max();
const qsizetype MinInvalid = MaxByteArraySize + 1; const qsizetype MinInvalid = MaxByteArraySize + 1 - sizeof(QByteArray::size_type);
addValidationColumns(); addValidationColumns();
addValidationData(MinInvalid); addValidationData(MinInvalid);
addValidationLargeData(MinInvalid, MaxInvalid); addValidationLargeData(MinInvalid, MaxInvalid);
// Chunked strings whose total overflows the limit, but each individual
// chunk doesn't. 0x5a for 32-bit, 0x5b for 64-bit.
char toolong[1 + sizeof(qsizetype)];
toolong[0] = sizeof(MinInvalid) > 4 ? 0x5b : 0x5a;
qToBigEndian(MinInvalid - 1, toolong + 1);
QTest::addRow("bytearray-2chunked+1-too-big-for-qbytearray-%llx", MinInvalid)
<< ("\x5f\x41z" + QByteArray(toolong, sizeof(toolong)) + '\xff')
<< 0 << CborErrorDataTooLarge;
toolong[0] |= 0x20;
QTest::addRow("string-2chunked+1-too-big-for-qbytearray-%llx", MinInvalid)
<< ("\x7f\x61z" + QByteArray(toolong, sizeof(toolong)) + '\xff')
<< 0 << CborErrorDataTooLarge;
// These tests say we have arrays and maps with very large item counts. // These tests say we have arrays and maps with very large item counts.
// They are meant to ensure we don't pre-allocate a lot of memory // They are meant to ensure we don't pre-allocate a lot of memory
// unnecessarily and possibly crash the application. The actual number of // unnecessarily and possibly crash the application. The actual number of