CBOR support: prevent overflowing QByteArray's max allocation

QByteArray doesn't like it.

Apply the same protection to QString, which we know uses the same
backend but uses elements twice as big. That means it can contain
slightly more than half as many elements, but exact half will suffice
for our needs.

Change-Id: Iaa63461109844e978376fffd15f9d4c7a9137856
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 783d574b932288b61f915b28d5b7b9c5a979f58e)
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

4 files changed, 71 insertions, 35 deletions

diff --git a/src/3rdparty/tinycbor/tests/parser/data.cpp b/src/3rdparty/tinycbor/tests/parser/data.cpp
index 0ab0e47be4..3523c32167 100644
--- a/src/3rdparty/tinycbor/tests/parser/data.cpp
+++ b/src/3rdparty/tinycbor/tests/parser/data.cpp
@@ -338,7 +338,7 @@ void addValidationColumns()
     QTest::addColumn<CborError>("expectedError");
 }
 
-void addValidationData()
+void addValidationData(size_t minInvalid = ~size_t(0))
 {
     // illegal numbers are future extension points
     QTest::newRow("illegal-number-in-unsigned-1") << raw("\x81\x1c") << 0 << CborErrorIllegalNumber;
@@ -488,26 +488,35 @@ void addValidationData()
     QTest::newRow("map-break-after-value-tag2") << raw("\x81\xbf\0\xd8\x20\xff") << 0 << CborErrorUnexpectedBreak;
 
     // check for pointer additions wrapping over the limit of the address space
-    CborError tooLargeOn32bit = (sizeof(void *) == 4) ? CborErrorDataTooLarge : CborErrorUnexpectedEOF;
+    auto wraparoundError = [minInvalid](uint64_t encodedSize) {
+        if (encodedSize > minInvalid)
+            return CborErrorDataTooLarge;
+        return CborErrorUnexpectedEOF;
+    };
+    constexpr uint64_t FourGB = UINT32_MAX + UINT64_C(1);
     // on 32-bit systems, this is a -1
-    QTest::newRow("bytearray-wraparound1") << raw("\x81\x5a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF;
-    QTest::newRow("string-wraparound1") << raw("\x81\x7a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF;
+    QTest::newRow("bytearray-wraparound1") << raw("\x81\x5a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX);
+    QTest::newRow("string-wraparound1") << raw("\x81\x7a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX);
     // on 32-bit systems, a 4GB addition could be dropped
-    QTest::newRow("bytearray-wraparound2") << raw("\x81\x5b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit;
-    QTest::newRow("string-wraparound2") << raw("\x81\x7b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit;
+    QTest::newRow("bytearray-wraparound2") << raw("\x81\x5b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB);
+    QTest::newRow("string-wraparound2") << raw("\x81\x7b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB);
     // on 64-bit systems, this could be a -1
-    QTest::newRow("bytearray-wraparound3") << raw("\x81\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit;
-    QTest::newRow("string-wraparound3") << raw("\x81\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit;
+    QTest::newRow("bytearray-wraparound3") << raw("\x81\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0
+                                           << wraparoundError(UINT64_MAX);
+    QTest::newRow("string-wraparound3") << raw("\x81\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0
+                                        << wraparoundError(UINT64_MAX);
 
     // ditto on chunks
-    QTest::newRow("bytearray-chunk-wraparound1") << raw("\x81\x5f\x5a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF;
-    QTest::newRow("string-chunk-wraparound1") << raw("\x81\x7f\x7a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF;
+    QTest::newRow("bytearray-chunk-wraparound1") << raw("\x81\x5f\x5a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX);
+    QTest::newRow("string-chunk-wraparound1") << raw("\x81\x7f\x7a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX);
     // on 32-bit systems, a 4GB addition could be dropped
-    QTest::newRow("bytearray-chunk-wraparound2") << raw("\x81\x5f\x5b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit;
-    QTest::newRow("string-chunk-wraparound2") << raw("\x81\x7f\x7b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit;
+    QTest::newRow("bytearray-chunk-wraparound2") << raw("\x81\x5f\x5b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB);
+    QTest::newRow("string-chunk-wraparound2") << raw("\x81\x7f\x7b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB);
     // on 64-bit systems, this could be a -1
-    QTest::newRow("bytearray-chunk-wraparound3") << raw("\x81\x5f\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit;
-    QTest::newRow("string-chunk-wraparound3") << raw("\x81\x7f\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit;
+    QTest::newRow("bytearray-chunk-wraparound3") << raw("\x81\x5f\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0
+                                                 << wraparoundError(UINT64_MAX);
+    QTest::newRow("string-chunk-wraparound3") << raw("\x81\x7f\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0
+                                              << wraparoundError(UINT64_MAX);
 
     QTest::newRow("eof-after-array") << raw("\x81") << 0 << CborErrorUnexpectedEOF;
     QTest::newRow("eof-after-array2") << raw("\x81\x78\x20") << 0 << CborErrorUnexpectedEOF;
diff --git a/src/corelib/serialization/qcborstream.cpp b/src/corelib/serialization/qcborstream.cpp
index dc00118cfd..de89d6954b 100644
--- a/src/corelib/serialization/qcborstream.cpp
+++ b/src/corelib/serialization/qcborstream.cpp
@@ -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.
@@ -39,6 +39,7 @@
 
 #include "qcborstream.h"
 
+#include <private/qbytearray_p.h>
 #include <private/qnumeric_p.h>
 #include <private/qutfcodec_p.h>
 #include <qbuffer.h>
@@ -2266,6 +2267,10 @@ bool QCborStreamReader::next(int maxRecursion)
     } else if (isString() || isByteArray()) {
         auto r = _readByteArray_helper();
         while (r.status == Ok) {
+            if (isString() && r.data.size() > MaxStringSize) {
+                d->handleError(CborErrorDataTooLarge);
+                break;
+            }
             if (isString() && !QUtf8::isValidUtf8(r.data, r.data.size()).isValidUtf8) {
                 d->handleError(CborErrorInvalidUtf8TextString);
                 break;
@@ -2548,15 +2553,23 @@ QCborStreamReader::StringResult<QString> QCborStreamReader::_readString_helper()
     result.status = r.status;
 
     if (r.status == Ok) {
-        QTextCodec::ConverterState cs;
-        result.data = QUtf8::convertToUnicode(r.data, r.data.size(), &cs);
-        if (cs.invalidChars == 0 && cs.remainingChars == 0)
-            return result;
+        // See QUtf8::convertToUnicode() a detailed explanation of why this
+        // conversion uses the same number of words or less.
+        CborError err = CborNoError;
+        if (r.data.size() > MaxStringSize) {
+            err = CborErrorDataTooLarge;
+        } else {
+            QTextCodec::ConverterState cs;
+            result.data = QUtf8::convertToUnicode(r.data, r.data.size(), &cs);
+            if (cs.invalidChars != 0 || cs.remainingChars != 0)
+                err = CborErrorInvalidUtf8TextString;
+        }
 
-        d->handleError(CborErrorInvalidUtf8TextString);
-        result.data.clear();
-        result.status = Error;
-        return result;
+        if (err) {
+            d->handleError(err);
+            result.data.clear();
+            result.status = Error;
+        }
     }
     return result;
 }
@@ -2584,6 +2597,10 @@ QCborStreamReader::StringResult<QByteArray> QCborStreamReader::_readByteArray_he
     qsizetype len = _currentStringChunkSize();
     if (len < 0)
         return result;
+    if (len > MaxByteArraySize) {
+        d->handleError(CborErrorDataTooLarge);
+        return result;
+    }
 
     result.data.resize(len);
     auto r = readStringChunk(result.data.data(), len);
diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp
index 6110c9a864..da5975e349 100644
--- a/src/corelib/serialization/qcborvalue.cpp
+++ b/src/corelib/serialization/qcborvalue.cpp
@@ -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.
@@ -46,6 +46,7 @@
 
 #include <qendian.h>
 #include <qlocale.h>
+#include <private/qbytearray_p.h>
 #include <private/qnumeric_p.h>
 #include <private/qsimd_p.h>
 
@@ -1525,6 +1526,8 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader)
         // and calculate the final size
         if (add_overflow(offset, increment, &newSize))
             return -1;
+        if (newSize > MaxByteArraySize)
+            return -1;
 
         // since usedData <= data.size(), this can't overflow
         usedData += increment;
@@ -1599,13 +1602,6 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader)
         setErrorInReader(reader, { QCborError::DataTooLarge });
     }
 
-    if (r.status == QCborStreamReader::Error) {
-        // There can only be errors if there was data to be read.
-        Q_ASSERT(e.flags & Element::HasByteData);
-        data.truncate(e.value);
-        return;
-    }
-
     // update size
     if (e.flags & Element::HasByteData) {
         auto b = new (dataPtr() + e.value) ByteData;
@@ -1617,6 +1613,21 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader)
             Q_ASSERT(e.type == QCborValue::String);
             e.flags |= Element::StringIsAscii;
         }
+
+        // check that this UTF-8 text string can be loaded onto a QString
+        if (e.type == QCborValue::String) {
+            if (Q_UNLIKELY(b->len > MaxStringSize)) {
+                setErrorInReader(reader, { QCborError::DataTooLarge });
+                r.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);
+        data.truncate(e.value);
+        return;
     }
 
     elements.append(e);
diff --git a/src/corelib/tools/qbytearray_p.h b/src/corelib/tools/qbytearray_p.h
index 6ebff739cd..a7d792a6ad 100644
--- a/src/corelib/tools/qbytearray_p.h
+++ b/src/corelib/tools/qbytearray_p.h
@@ -56,10 +56,9 @@
 
 QT_BEGIN_NAMESPACE
 
-enum {
-    // Define as enum to force inlining. Don't expose MaxAllocSize in a public header.
-    MaxByteArraySize = MaxAllocSize - sizeof(std::remove_pointer<QByteArray::DataPtr>::type)
-};
+// -1 because of the terminating NUL
+constexpr qsizetype MaxByteArraySize = MaxAllocSize - sizeof(std::remove_pointer<QByteArray::DataPtr>::type) - 1;
+constexpr qsizetype MaxStringSize = (MaxAllocSize - sizeof(std::remove_pointer<QByteArray::DataPtr>::type)) / 2 - 1;
 
 QT_END_NAMESPACE