1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QCborValue: further clamp down on memory allocation for CBOR streams

Browse Source 
We were allowing up to 1 million elements for each array, which meant 16
MB (32 for maps), and we considered that sufficient. However, because we
do allow up to 1024 levels of recursion, the memory consumption was
actually limited to 16 GB (32 for maps), which is a bit too high for
64-bit applications, and definitely too high for 32-bit ones.

So further clamp down, to a mere 16k elements on 32-bit and 64k on 64-
bit, which limits the memory use to 256 MB on 32-bit and 1 GB on 64-bit.
And additionally apply this exact limit to maps, instead of allowing
them to double the size.

As before, this does not limit the size of non-corrupt streams. This
only limits the initial memory pre-allocation.

Pick-to: 6.2 6.3 6.4
Fixes: QTBUG-104718
Change-Id: I89c4eb48af38408daa7cfffd16fdcb34f08c1949
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Thiago Macieira 2022-07-01 12:27:46 -07:00
parent 65adfd5ec5
commit f6e6ae092d
1 changed files with 28 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/corelib/serialization/qcborvalue.cpp
View File

@ -25,6 +25,21 @@
QT_BEGIN_NAMESPACE
// Worst case memory allocation for a corrupt stream: 256 MB for 32-bit, 1 GB for 64-bit
static constexpr quint64 MaxAcceptableMemoryUse = (sizeof(void*) == 4 ? 256 : 1024) * 1024 * 1024;
// Internal limits to ensure we don't blow up the memory when parsing a corrupt
// (possibly crafted to exploit) CBOR stream. The recursion impacts both the
// maps/arrays we'll open when parsing and the thread's stack, as the parser is
// itself recursive. If someone really needs more than 1024 layers of nesting,
// they probably have a weird use-case for which custom parsing and
// serialisation code would make sense. The limit on element count is the
// preallocated limit: if the stream does actually have more elements, we will
// grow the container.
Q_DECL_UNUSED static constexpr int MaximumRecursionDepth = 1024;
Q_DECL_UNUSED static constexpr quint64 MaximumPreallocatedElementCount =
MaxAcceptableMemoryUse / MaximumRecursionDepth / sizeof(QtCbor::Element) - 1;
/*!
\class QCborValue
\inmodule QtCore
@ -1458,6 +1473,17 @@ static Element decodeBasicValueFromCbor(QCborStreamReader &reader)
return e;
}
// Clamp allocation to avoid crashing due to corrupt stream. This also
// ensures we never overflow qsizetype. The returned length is doubled for Map
// entries to account for key-value pairs.
static qsizetype clampedContainerLength(const QCborStreamReader &reader)
{
int mapShift = reader.isMap() ? 1 : 0;
quint64 shiftedMaxElements = MaximumPreallocatedElementCount >> mapShift;
qsizetype len = qsizetype(qMin(reader.length(), shiftedMaxElements));
return len << mapShift;
}
static inline QCborContainerPrivate *createContainerFromCbor(QCborStreamReader &reader, int remainingRecursionDepth)
{
if (Q_UNLIKELY(remainingRecursionDepth == 0)) {
@ -1466,18 +1492,11 @@ static inline QCborContainerPrivate *createContainerFromCbor(QCborStreamReader &
}
QCborContainerPrivate *d = nullptr;
int mapShift = reader.isMap() ? 1 : 0;
if (reader.isLengthKnown()) {
quint64 len = reader.length();
// Clamp allocation to 1M elements (avoids crashing due to corrupt
// stream or loss of precision when converting from quint64 to
// QList::size_type).
len = qMin(len, quint64(1024 * 1024 - 1));
if (len) {
if (qsizetype len = clampedContainerLength(reader)) {
d = new QCborContainerPrivate;
d->ref.storeRelaxed(1);
d->elements.reserve(qsizetype(len) << mapShift);
d->elements.reserve(len);
}
} else {
d = new QCborContainerPrivate;
@ -2344,8 +2363,6 @@ QCborValueRef QCborValue::operator[](qint64 key)
}
#if QT_CONFIG(cborstreamreader)
enum { MaximumRecursionDepth = 1024 };
/*!
Decodes one item from the CBOR stream found in \a reader and returns the
equivalent representation. This function is recursive: if the item is a map