diff options
| -rw-r--r-- | src/corelib/serialization/qcborstreamreader.cpp | 28 | ||||
| -rw-r--r-- | src/corelib/serialization/qcborvalue.cpp | 82 | ||||
| -rw-r--r-- | src/corelib/serialization/qcborvalue_p.h | 10 | ||||
| -rw-r--r-- | tests/auto/corelib/serialization/cborlargedatavalidation.cpp | 15 | ||||
| -rw-r--r-- | tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp | 6 |
5 files changed, 82 insertions, 59 deletions
diff --git a/src/corelib/serialization/qcborstreamreader.cpp b/src/corelib/serialization/qcborstreamreader.cpp index ddc93e469f..d097979bda 100644 --- a/src/corelib/serialization/qcborstreamreader.cpp +++ b/src/corelib/serialization/qcborstreamreader.cpp @@ -681,6 +681,7 @@ public: bool isPlainPointer() const { return maxlen_or_type >= 0; } }; + static QCborStreamReader::StringResultCode appendStringChunk(QCborStreamReader &reader, QByteArray *data); QCborStreamReader::StringResult<qsizetype> readStringChunk(ReadStringChunk params); bool ensureStringIteration(); }; @@ -1479,6 +1480,21 @@ QCborStreamReader::readStringChunk(char *ptr, qsizetype maxlen) return r; } +// used by qcborvalue.cpp +QCborStreamReader::StringResultCode qt_cbor_append_string_chunk(QCborStreamReader &reader, QByteArray *data) +{ + return QCborStreamReaderPrivate::appendStringChunk(reader, data); +} + +inline QCborStreamReader::StringResultCode +QCborStreamReaderPrivate::appendStringChunk(QCborStreamReader &reader, QByteArray *data) +{ + auto status = reader.d->readStringChunk(data).status; + if (status == QCborStreamReader::EndOfString && reader.lastError() == QCborError::NoError) + reader.preparse(); + return status; +} + Q_NEVER_INLINE QCborStreamReader::StringResult<qsizetype> QCborStreamReaderPrivate::readStringChunk(ReadStringChunk params) { @@ -1547,17 +1563,23 @@ QCborStreamReaderPrivate::readStringChunk(ReadStringChunk params) ptr = params.ptr; } else if (params.isByteArray()) { // See note above on having ensured there is enough incoming data. + auto oldSize = params.array->size(); + auto newSize = oldSize; + if (add_overflow<decltype(newSize)>(oldSize, toRead, &newSize)) { + handleError(CborErrorDataTooLarge); + return result; + } try { - params.array->resize(toRead); + params.array->resize(newSize); } catch (const std::bad_alloc &) { // the distinction between DataTooLarge and OOM is mostly for // compatibility with Qt 5; in Qt 6, we could consider everything // to be OOM. - handleError(toRead > MaxByteArraySize ? CborErrorDataTooLarge: CborErrorOutOfMemory); + handleError(newSize > MaxByteArraySize ? CborErrorDataTooLarge: CborErrorOutOfMemory); return result; } - ptr = const_cast<char *>(params.array->constData()); + ptr = const_cast<char *>(params.array->constData()) + oldSize; } if (device) { diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index c7a2198644..f27ef94b4a 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -1556,31 +1556,37 @@ inline void QCborContainerPrivate::setErrorInReader(QCborStreamReader &reader, Q qt_cbor_stream_set_error(reader.d.data(), error); } +extern QCborStreamReader::StringResultCode qt_cbor_append_string_chunk(QCborStreamReader &reader, QByteArray *data); + void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) { - auto addByteData_local = [this](QByteArray::size_type len) -> qint64 { - // this duplicates a lot of addByteData, but with overflow checking - QByteArray::size_type newSize; - QByteArray::size_type increment = sizeof(QtCbor::ByteData); - QByteArray::size_type alignment = alignof(QtCbor::ByteData); - QByteArray::size_type offset = data.size(); - - // calculate the increment we want - if (add_overflow(increment, len, &increment)) - return -1; - - // align offset - if (add_overflow(offset, alignment - 1, &offset)) - return -1; - offset &= ~(alignment - 1); - - // and calculate the final size - if (add_overflow(offset, increment, &newSize)) - return -1; - if (newSize > MaxByteArraySize) + // The use of size_t means none of the operations here can overflow because + // all inputs are less than half SIZE_MAX. + auto addByteData_local = [this](size_t len) -> qint64 { + constexpr size_t EstimatedOverhead = 16; + constexpr size_t MaxMemoryIncrement = 16384; + size_t offset = data.size(); + + // add space for aligned ByteData (this can't overflow) + offset += sizeof(QtCbor::ByteData) + alignof(QtCbor::ByteData); + offset &= ~(alignof(QtCbor::ByteData) - 1); + if (offset > size_t(MaxByteArraySize)) return -1; - data.resize(newSize); + // and calculate the size we want to have + size_t newCapacity = offset + len; // can't overflow + if (len > MaxMemoryIncrement - EstimatedOverhead) { + // there's a non-zero chance that we won't need this memory at all, + // so capa how much we allocate + newCapacity = offset + MaxMemoryIncrement - EstimatedOverhead; + } + if (newCapacity > size_t(MaxByteArraySize)) { + // this may cause an allocation failure + newCapacity = MaxByteArraySize; + } + if (newCapacity > size_t(data.capacity())) + data.reserve(newCapacity); + data.resize(offset + sizeof(QtCbor::ByteData)); return offset; }; auto dataPtr = [this]() { @@ -1617,42 +1623,32 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) // read chunks bool isAscii = (e.type == QCborValue::String); - auto r = reader.readStringChunk(dataPtr() + e.value + sizeof(ByteData), len); - while (r.status == QCborStreamReader::Ok) { + QCborStreamReader::StringResultCode status = qt_cbor_append_string_chunk(reader, &data); + while (status == QCborStreamReader::Ok) { if (e.type == QCborValue::String && len) { // verify UTF-8 string validity auto utf8result = QUtf8::isValidUtf8(QByteArrayView(dataPtr(), data.size()).last(len)); if (!utf8result.isValidUtf8) { - r.status = QCborStreamReader::Error; + status = QCborStreamReader::Error; setErrorInReader(reader, { QCborError::InvalidUtf8String }); break; } isAscii = isAscii && utf8result.isValidAscii; } - // allocate space for the next chunk rawlen = reader.currentStringChunkSize(); len = rawlen; if (len == rawlen) { - auto oldSize = data.size(); - auto newSize = oldSize; - if (!add_overflow(newSize, len, &newSize) && newSize < MaxByteArraySize) { - if (newSize != oldSize) - data.resize(newSize); - - // read the chunk - r = reader.readStringChunk(dataPtr() + oldSize, len); - continue; - } + status = qt_cbor_append_string_chunk(reader, &data); + } else { + // error + status = QCborStreamReader::Error; + setErrorInReader(reader, { QCborError::DataTooLarge }); } - - // error - r.status = QCborStreamReader::Error; - setErrorInReader(reader, { QCborError::DataTooLarge }); } // update size - if (r.status == QCborStreamReader::EndOfString && e.flags & Element::HasByteData) { + if (status == QCborStreamReader::EndOfString) { auto b = new (dataPtr() + e.value) ByteData; b->len = data.size() - e.value - int(sizeof(*b)); usedData += b->len; @@ -1667,14 +1663,12 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) if (e.type == QCborValue::String) { if (Q_UNLIKELY(b->len > MaxStringSize)) { setErrorInReader(reader, { QCborError::DataTooLarge }); - r.status = QCborStreamReader::Error; + status = QCborStreamReader::Error; } } } - if (r.status == QCborStreamReader::Error) { - // There can only be errors if there was data to be read. - Q_ASSERT(e.flags & Element::HasByteData); + if (status == QCborStreamReader::Error) { data.truncate(e.value); return; } diff --git a/src/corelib/serialization/qcborvalue_p.h b/src/corelib/serialization/qcborvalue_p.h index d68d44e7be..5c8f244ad8 100644 --- a/src/corelib/serialization/qcborvalue_p.h +++ b/src/corelib/serialization/qcborvalue_p.h @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2018 Intel Corporation. +** Copyright (C) 2020 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -53,6 +53,10 @@ #include "qcborvalue.h" +#if QT_CONFIG(cborstreamreader) +# include "qcborstreamreader.h" +#endif + #include <private/qglobal_p.h> #include <private/qstringconverter_p.h> @@ -414,9 +418,11 @@ public: elements.remove(idx); } - void decodeValueFromCbor(QCborStreamReader &reader, int remainiingStackDepth); +#if QT_CONFIG(cborstreamreader) + void decodeValueFromCbor(QCborStreamReader &reader, int remainingStackDepth); void decodeStringFromCbor(QCborStreamReader &reader); static inline void setErrorInReader(QCborStreamReader &reader, QCborError error); +#endif }; QT_END_NAMESPACE diff --git a/tests/auto/corelib/serialization/cborlargedatavalidation.cpp b/tests/auto/corelib/serialization/cborlargedatavalidation.cpp index 64191db1ef..c5cf1dc04e 100644 --- a/tests/auto/corelib/serialization/cborlargedatavalidation.cpp +++ b/tests/auto/corelib/serialization/cborlargedatavalidation.cpp @@ -87,25 +87,28 @@ void addValidationLargeData(qsizetype minInvalid, qsizetype maxInvalid) toolong[0] = sizeof(v) > 4 ? 0x5b : 0x5a; qToBigEndian(v, toolong + 1); + bool overflows = v > std::numeric_limits<qsizetype>::max() - 1 - qsizetype(sizeof(v)); + CborError err = overflows ? CborErrorDataTooLarge : CborErrorUnexpectedEOF; + QTest::addRow("bytearray-too-big-for-qbytearray-%llx", v) - << QByteArray(toolong, sizeof(toolong)) << 0 << CborErrorUnexpectedEOF; + << QByteArray(toolong, sizeof(toolong)) << 0 << err; QTest::addRow("bytearray-chunked-too-big-for-qbytearray-%llx", v) << ('\x5f' + QByteArray(toolong, sizeof(toolong)) + '\xff') - << 0 << CborErrorUnexpectedEOF; + << 0 << err; QTest::addRow("bytearray-2chunked-too-big-for-qbytearray-%llx", v) << ("\x5f\x40" + QByteArray(toolong, sizeof(toolong)) + '\xff') - << 0 << CborErrorUnexpectedEOF; + << 0 << err; toolong[0] |= 0x20; // QCborStreamReader::readString copies to a QByteArray first QTest::addRow("string-too-big-for-qbytearray-%llx", v) - << QByteArray(toolong, sizeof(toolong)) << 0 << CborErrorUnexpectedEOF; + << QByteArray(toolong, sizeof(toolong)) << 0 << err; QTest::addRow("string-chunked-too-big-for-qbytearray-%llx", v) << ('\x7f' + QByteArray(toolong, sizeof(toolong)) + '\xff') - << 0 << CborErrorUnexpectedEOF; + << 0 << err; QTest::addRow("string-2chunked-too-big-for-qbytearray-%llx", v) << ("\x7f\x60" + QByteArray(toolong, sizeof(toolong)) + '\xff') - << 0 << CborErrorUnexpectedEOF; + << 0 << err; } } diff --git a/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp b/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp index 64d4451f08..4ec5095f1c 100644 --- a/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp +++ b/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp @@ -2007,11 +2007,11 @@ void tst_QCborValue::validation_data() 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; + << 0 << CborErrorUnexpectedEOF; toolong[0] |= 0x20; QTest::addRow("string-2chunked+1-too-big-for-qbytearray-%llx", MinInvalid) << ("\x7f\x61z" + QByteArray(toolong, sizeof(toolong)) + '\xff') - << 0 << CborErrorDataTooLarge; + << 0 << CborErrorUnexpectedEOF; // 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 @@ -2042,7 +2042,6 @@ void tst_QCborValue::validation() QCborParserError parserError; QCborValue decoded = QCborValue::fromCbor(data, &parserError); - if (parserError.error != QCborError::DataTooLarge) // ### temporary!! QCOMPARE(parserError.error, error); if (data.startsWith('\x81')) { @@ -2050,7 +2049,6 @@ void tst_QCborValue::validation() char *ptr = const_cast<char *>(data.constData()); QByteArray mid = QByteArray::fromRawData(ptr + 1, data.size() - 1); decoded = QCborValue::fromCbor(mid, &parserError); - if (parserError.error != QCborError::DataTooLarge) // ### temporary!! QCOMPARE(parserError.error, error); } } |