QBitArray: avoid overflow in size-to-storage calculations

Unlike other containers, a QBitArray's size() is not limited by
storage, but, esp. on 32-bit platforms, its size_type: A INT_MAX
size() QBitArray only requires 256MiB of storage.

So we can't rely on "won't happen in practice" here and need to avoid
the potential UB (signed overflow) in the (size + 7) / 8
logical-to-storage-size calculation by using unsigned arithmetic.

Use the opportunity to Extract Methods storage_size() and
allocation_size(), which differ by one (d[[0] contains the size() mod
8), making it clear what's what.

[ChangeLog][QtCore][QBitArray] Fixed a bug with QBitArrays whose
size() came within 7 of the size_type's maximum.

Pick-to: 6.7 6.6 6.5 6.2 5.15
Change-Id: I5d94bae9c9c210ba1e36f8cf03609125c81bd15d
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 59 insertions, 4 deletions

diff --git a/src/corelib/tools/qbitarray.cpp b/src/corelib/tools/qbitarray.cpp
index fed2bb6bd2..cc3b8a15cf 100644
--- a/src/corelib/tools/qbitarray.cpp
+++ b/src/corelib/tools/qbitarray.cpp
@@ -104,12 +104,23 @@ QT_BEGIN_NAMESPACE
  *    inline qsizetype size() const { return (d.size() << 3) - *d.constData(); }
  */
 
+static constexpr qsizetype storage_size(qsizetype size)
+{
+    // avoid overflow when adding 7, by doing the arithmetic in unsigned space:
+    return qsizetype((size_t(size) + 7) / 8);
+}
+
+static constexpr qsizetype allocation_size(qsizetype size)
+{
+    return size <= 0 ? 0 : storage_size(size) + 1;
+}
+
 /*!
     Constructs a bit array containing \a size bits. The bits are
     initialized with \a value, which defaults to false (0).
 */
 QBitArray::QBitArray(qsizetype size, bool value)
-    : d(size <= 0 ? 0 : 1 + (size + 7) / 8, Qt::Uninitialized)
+    : d(allocation_size(size), Qt::Uninitialized)
 {
     Q_ASSERT_X(size >= 0, "QBitArray::QBitArray", "Size must be greater than or equal to 0.");
     if (size <= 0)
@@ -187,7 +198,7 @@ void QBitArray::resize(qsizetype size)
         d.resize(0);
     } else {
         qsizetype s = d.size();
-        d.resize(1 + (size + 7) / 8);
+        d.resize(allocation_size(size));
         uchar *c = reinterpret_cast<uchar *>(d.data());
         if (size > (s << 3))
             memset(c + s, 0, d.size() - s);
@@ -292,7 +303,7 @@ QBitArray QBitArray::fromBits(const char *data, qsizetype size)
     QBitArray result;
     if (size == 0)
         return result;
-    qsizetype nbytes = (size + 7) / 8;
+    qsizetype nbytes = storage_size(size);
 
     result.d = QByteArray(nbytes + 1, Qt::Uninitialized);
     char *bits = result.d.data();
@@ -922,7 +933,7 @@ QDataStream &operator>>(QDataStream &in, QBitArray &ba)
     }
 
     const qsizetype Step = 8 * 1024 * 1024;
-    qsizetype totalBytes = (len + 7) / 8;
+    const qsizetype totalBytes = storage_size(len);
     qsizetype allocated = 0;
 
     while (allocated < totalBytes) {
diff --git a/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp b/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp
index 8fd209c7e2..7f33c6060b 100644
--- a/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp
+++ b/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp
@@ -7,6 +7,9 @@
 
 #include "qbitarray.h"
 
+#include <QtCore/qelapsedtimer.h>
+#include <QtCore/qscopeguard.h>
+
 /**
  * Helper function to initialize a bitarray from a string
  */
@@ -36,6 +39,7 @@ class tst_QBitArray : public QObject
 {
     Q_OBJECT
 private slots:
+    void canHandleIntMaxBits();
     void size_data();
     void size();
     void countBits_data();
@@ -81,6 +85,46 @@ private slots:
     void toUInt32();
 };
 
+void tst_QBitArray::canHandleIntMaxBits()
+{
+    QElapsedTimer timer;
+    timer.start();
+    const auto print = qScopeGuard([&] {
+        qDebug("Function took %lldms", qlonglong(timer.elapsed()));
+    });
+
+    try {
+        constexpr int Size1 = INT_MAX - 2;
+        constexpr int Size2 = Size1 + 2;
+
+        QBitArray ba(Size1, true);
+        QCOMPARE(ba.size(), Size1);
+        QCOMPARE(ba.at(Size1 - 1), true);
+
+        ba.resize(Size2);
+        QCOMPARE(ba.size(), Size2);
+        QCOMPARE(ba.at(Size1 - 1), true);
+        QCOMPARE(ba.at(Size1),     false);
+        QCOMPARE(ba.at(Size2 - 1), false);
+
+        QByteArray serialized;
+        {
+            QDataStream ds(&serialized, QIODevice::WriteOnly);
+            ds << ba;
+            QCOMPARE(ds.status(), QDataStream::Status::Ok);
+        }
+        {
+            QDataStream ds(serialized);
+            QBitArray ba2;
+            ds >> ba2;
+            QCOMPARE(ds.status(), QDataStream::Status::Ok);
+            QCOMPARE(ba, ba2);
+        }
+    } catch (const std::bad_alloc &) {
+        QSKIP("Failed to allocate sufficient memory");
+    }
+}
+
 void tst_QBitArray::size_data()
 {
     //create the testtable instance and define the elements