From f77caf4c605904f661f8520f54bfab42be0c01a5 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 16 Nov 2020 12:36:12 -0800 Subject: [PATCH] QString/QByteArray: add missing Q_CHECK_PTR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So these two classes throw when trying to allocate silly sizes or in OOM conditions. We probably want to move these Q_CHECK_POINTER into QTypedArrayData but I didn't want to do that in this commit. Task-number: QTBUG-88256 Task-number: QTBUG-88253 Change-Id: Ifc61bb80b9bf48a386abfffd1648176111770174 Reviewed-by: Andrei Golubev Reviewed-by: Mårten Nordheim (cherry picked from commit aab5c8e5486a6484feddfae0b04fd39fd244d9b9) Reviewed-by: Thiago Macieira --- src/corelib/text/qbytearray.cpp | 7 +++++++ src/corelib/text/qstring.cpp | 8 ++++++++ src/corelib/tools/qarraydata.cpp | 3 +++ src/corelib/tools/qarraydataops.h | 2 ++ 4 files changed, 20 insertions(+) diff --git a/src/corelib/text/qbytearray.cpp b/src/corelib/text/qbytearray.cpp index 6de923fe292..21f187012b9 100644 --- a/src/corelib/text/qbytearray.cpp +++ b/src/corelib/text/qbytearray.cpp @@ -1668,6 +1668,7 @@ QByteArray::QByteArray(const char *data, qsizetype size) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(size), size); + Q_CHECK_PTR(d.data()); memcpy(d.data(), data, size); d.data()[size] = '\0'; } @@ -1686,6 +1687,7 @@ QByteArray::QByteArray(qsizetype size, char ch) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(size), size); + Q_CHECK_PTR(d.data()); memset(d.data(), ch, size); d.data()[size] = '\0'; } @@ -1703,6 +1705,7 @@ QByteArray::QByteArray(qsizetype size, Qt::Initialization) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(size), size); + Q_CHECK_PTR(d.data()); d.data()[size] = '\0'; } } @@ -1766,6 +1769,7 @@ void QByteArray::reallocData(qsizetype alloc, QArrayData::AllocationOption optio if (d->needsDetach() || cannotUseReallocate) { DataPointer dd(Data::allocate(alloc, option), qMin(alloc, d.size)); + Q_CHECK_PTR(dd.data()); if (dd.size > 0) ::memcpy(dd.data(), d.data(), dd.size); dd.data()[dd.size] = 0; @@ -1782,6 +1786,7 @@ void QByteArray::reallocGrowData(qsizetype n) if (d->needsDetach()) { DataPointer dd(DataPointer::allocateGrow(d, n, QArrayData::GrowsAtEnd)); + Q_CHECK_PTR(dd.data()); dd->copyAppend(d.data(), d.data() + d.size); dd.data()[dd.size] = 0; d = dd; @@ -2000,6 +2005,7 @@ QByteArray &QByteArray::insert(qsizetype i, QByteArrayView data) DataPointer detached{}; // construction is free if (d->needsDetach() || i + size - d->size > d.freeSpaceAtEnd()) { detached = DataPointer::allocateGrow(d, i + size - d->size, Data::GrowsAtEnd); + Q_CHECK_PTR(detached.data()); detached->copyAppend(d.constBegin(), d.constEnd()); d.swap(detached); } @@ -2081,6 +2087,7 @@ QByteArray &QByteArray::insert(qsizetype i, qsizetype count, char ch) // handle this specially, as QArrayDataOps::insert() doesn't handle out of bounds positions if (d->needsDetach() || i + count - d->size > d.freeSpaceAtEnd()) { DataPointer detached(DataPointer::allocateGrow(d, i + count - d->size, Data::GrowsAtEnd)); + Q_CHECK_PTR(detached.data()); detached->copyAppend(d.constBegin(), d.constEnd()); d.swap(detached); } diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp index 7bbbc48cfba..ca16fc85816 100644 --- a/src/corelib/text/qstring.cpp +++ b/src/corelib/text/qstring.cpp @@ -2261,6 +2261,7 @@ QString::QString(const QChar *unicode, qsizetype size) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(size), size); + Q_CHECK_PTR(d.data()); memcpy(d.data(), unicode, size * sizeof(QChar)); d.data()[size] = '\0'; } @@ -2279,6 +2280,7 @@ QString::QString(qsizetype size, QChar ch) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(size), size); + Q_CHECK_PTR(d.data()); d.data()[size] = '\0'; char16_t *i = d.data() + size; char16_t *b = d.data(); @@ -2300,6 +2302,7 @@ QString::QString(qsizetype size, Qt::Initialization) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(size), size); + Q_CHECK_PTR(d.data()); d.data()[size] = '\0'; } } @@ -2317,6 +2320,7 @@ QString::QString(qsizetype size, Qt::Initialization) QString::QString(QChar ch) { d = DataPointer(Data::allocate(1), 1); + Q_CHECK_PTR(d.data()); d.data()[0] = ch.unicode(); d.data()[1] = '\0'; } @@ -2507,6 +2511,7 @@ void QString::reallocData(qsizetype alloc, QArrayData::AllocationOption option) if (d->needsDetach() || cannotUseReallocate) { DataPointer dd(Data::allocate(alloc, option), qMin(alloc, d.size)); + Q_CHECK_PTR(dd.data()); if (dd.size > 0) ::memcpy(dd.data(), d.data(), dd.size * sizeof(QChar)); dd.data()[dd.size] = 0; @@ -2523,6 +2528,7 @@ void QString::reallocGrowData(qsizetype n) if (d->needsDetach()) { DataPointer dd(DataPointer::allocateGrow(d, n, QArrayData::GrowsAtEnd)); + Q_CHECK_PTR(dd.data()); dd->copyAppend(d.data(), d.data() + d.size); dd.data()[dd.size] = 0; d = dd; @@ -2726,6 +2732,7 @@ QString& QString::insert(qsizetype i, const QChar *unicode, qsizetype size) DataPointer detached{}; // construction is free if (d->needsDetach() || i + size - d->size > d.freeSpaceAtEnd()) { detached = DataPointer::allocateGrow(d, i + size - d->size, Data::GrowsAtEnd); + Q_CHECK_PTR(detached.data()); detached->copyAppend(d.constBegin(), d.constEnd()); d.swap(detached); } @@ -5118,6 +5125,7 @@ QString QString::fromLatin1(QByteArrayView ba) d = DataPointer::fromRawData(&_empty, 0); } else { d = DataPointer(Data::allocate(ba.size()), ba.size()); + Q_CHECK_PTR(d.data()); d.data()[ba.size()] = '\0'; char16_t *dst = d.data(); diff --git a/src/corelib/tools/qarraydata.cpp b/src/corelib/tools/qarraydata.cpp index 5feb1ac8f68..8ca315024d8 100644 --- a/src/corelib/tools/qarraydata.cpp +++ b/src/corelib/tools/qarraydata.cpp @@ -235,6 +235,9 @@ QArrayData::reallocateUnaligned(QArrayData *data, void *dataPointer, const qsizetype headerSize = sizeof(QArrayData); qsizetype allocSize = calculateBlockSize(capacity, objectSize, headerSize, option); + if (Q_UNLIKELY(allocSize < 0)) + return qMakePair(nullptr, nullptr); + const qptrdiff offset = dataPointer ? reinterpret_cast(dataPointer) - reinterpret_cast(data) : headerSize; diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h index cf054a089fd..0c7703c5880 100644 --- a/src/corelib/tools/qarraydataops.h +++ b/src/corelib/tools/qarraydataops.h @@ -286,6 +286,7 @@ public: void reallocate(qsizetype alloc, QArrayData::AllocationOption option) { auto pair = Data::reallocateUnaligned(this->d, this->ptr, alloc, option); + Q_CHECK_PTR(pair.second); Q_ASSERT(pair.first != nullptr); this->d = pair.first; this->ptr = pair.second; @@ -849,6 +850,7 @@ public: void reallocate(qsizetype alloc, QArrayData::AllocationOption option) { auto pair = Data::reallocateUnaligned(this->d, this->ptr, alloc, option); + Q_CHECK_PTR(pair.second); Q_ASSERT(pair.first != nullptr); this->d = pair.first; this->ptr = pair.second;