From 1e18f2c4a647923fc66a3e3204fcccd88a2960a6 Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@qt.io>
Date: Tue, 26 Mar 2019 09:40:03 +0100
Subject: Check for numeric limits when growing SharedInternalClassDataPrivate

We can effectively only deal with values of < 2GB for m_alloc *
sizeof(Data). This is not much more than the values seen in the wild.

Change-Id: Ia6972df33d34a320b5b087d38db81aae24ce5bbe
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
---
 src/qml/jsruntime/qv4internalclass.cpp | 28 ++++++++++++++++++----------
 src/qml/jsruntime/qv4internalclass_p.h |  8 ++++++--
 src/qml/jsruntime/qv4memberdata.cpp    | 21 +++++++++++++++++----
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp
index 3fb2465a45..a10fda79f2 100644
--- a/src/qml/jsruntime/qv4internalclass.cpp
+++ b/src/qml/jsruntime/qv4internalclass.cpp
@@ -145,7 +145,7 @@ SharedInternalClassDataPrivate<PropertyKey>::SharedInternalClassDataPrivate(cons
       data(nullptr)
 {
     if (other.alloc()) {
-        int s = other.size();
+        const uint s = other.size();
         data = MemberData::allocate(engine, other.alloc(), other.data);
         setSize(s);
     }
@@ -164,8 +164,8 @@ SharedInternalClassDataPrivate<PropertyKey>::SharedInternalClassDataPrivate(cons
 
 void SharedInternalClassDataPrivate<PropertyKey>::grow()
 {
-    uint a = alloc() * 2;
-    int s = size();
+    const uint a = alloc() * 2;
+    const uint s = size();
     data = MemberData::allocate(engine, a, data);
     setSize(s);
     Q_ASSERT(alloc() >= a);
@@ -209,10 +209,11 @@ SharedInternalClassDataPrivate<PropertyAttributes>::SharedInternalClassDataPriva
         const SharedInternalClassDataPrivate<PropertyAttributes> &other, uint pos,
         PropertyAttributes value)
     : refcount(1),
-      m_alloc(pos + 8),
+      m_alloc(qMin(other.m_alloc, pos + 8)),
       m_size(pos + 1),
       m_engine(other.m_engine)
 {
+    Q_ASSERT(m_size <= m_alloc);
     m_engine->memoryManager->changeUnmanagedHeapSizeUsage(m_alloc * sizeof(PropertyAttributes));
     data = new PropertyAttributes[m_alloc];
     if (other.data)
@@ -244,22 +245,29 @@ SharedInternalClassDataPrivate<PropertyAttributes>::~SharedInternalClassDataPriv
 }
 
 void SharedInternalClassDataPrivate<PropertyAttributes>::grow() {
+    uint alloc;
     if (!m_alloc) {
-        m_alloc = 4;
-        m_engine->memoryManager->changeUnmanagedHeapSizeUsage(
-                2 * m_alloc * sizeof(PropertyAttributes));
+        alloc = 8;
+        m_engine->memoryManager->changeUnmanagedHeapSizeUsage(alloc * sizeof(PropertyAttributes));
     } else {
+        // yes, signed. We don't want to deal with stuff > 2G
+        const uint currentSize = m_alloc * sizeof(PropertyAttributes);
+        if (currentSize < uint(std::numeric_limits<int>::max() / 2))
+            alloc = m_alloc * 2;
+        else
+            alloc = std::numeric_limits<int>::max() / sizeof(PropertyAttributes);
+
         m_engine->memoryManager->changeUnmanagedHeapSizeUsage(
-                m_alloc * sizeof(PropertyAttributes));
+                (alloc - m_alloc) * sizeof(PropertyAttributes));
     }
 
-    auto *n = new PropertyAttributes[m_alloc * 2];
+    auto *n = new PropertyAttributes[alloc];
     if (data) {
         memcpy(n, data, m_alloc*sizeof(PropertyAttributes));
         delete [] data;
     }
     data = n;
-    m_alloc *= 2;
+    m_alloc = alloc;
 }
 
 namespace Heap {
diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index 121238c555..42b61218a5 100644
--- a/src/qml/jsruntime/qv4internalclass_p.h
+++ b/src/qml/jsruntime/qv4internalclass_p.h
@@ -247,8 +247,12 @@ struct SharedInternalClassData {
         Q_ASSERT(pos == d->size());
         if (pos == d->alloc())
             d->grow();
-        d->setSize(d->size() + 1);
-        d->set(pos, value);
+        if (pos >= d->alloc()) {
+            qBadAlloc();
+        } else {
+            d->setSize(d->size() + 1);
+            d->set(pos, value);
+        }
     }
 
     void set(uint pos, T value) {
diff --git a/src/qml/jsruntime/qv4memberdata.cpp b/src/qml/jsruntime/qv4memberdata.cpp
index 246f857643..f327c85001 100644
--- a/src/qml/jsruntime/qv4memberdata.cpp
+++ b/src/qml/jsruntime/qv4memberdata.cpp
@@ -69,12 +69,25 @@ Heap::MemberData *MemberData::allocate(ExecutionEngine *e, uint n, Heap::MemberD
     size_t alloc = MemoryManager::align(sizeof(Heap::MemberData) + (n - 1)*sizeof(Value));
     // round up to next power of two to avoid quadratic behaviour for very large objects
     alloc = nextPowerOfTwo(alloc);
-    Heap::MemberData *m = e->memoryManager->allocManaged<MemberData>(alloc);
-    if (old)
+
+    // The above code can overflow in a number of interesting ways. All of those are unsigned,
+    // and therefore defined behavior. Still, apply some sane bounds.
+    if (alloc > std::numeric_limits<int>::max())
+        alloc = std::numeric_limits<int>::max();
+
+    Heap::MemberData *m;
+    if (old) {
+        const size_t oldSize = sizeof(Heap::MemberData) + (old->values.size - 1) * sizeof(Value);
+        if (oldSize > alloc)
+            alloc = oldSize;
+        m = e->memoryManager->allocManaged<MemberData>(alloc);
         // no write barrier required here
-        memcpy(m, old, sizeof(Heap::MemberData) + (old->values.size - 1) * sizeof(Value));
-    else
+        memcpy(m, old, oldSize);
+    } else {
+        m = e->memoryManager->allocManaged<MemberData>(alloc);
         m->init();
+    }
+
     m->values.alloc = static_cast<uint>((alloc - sizeof(Heap::MemberData) + sizeof(Value))/sizeof(Value));
     m->values.size = m->values.alloc;
     return m;
-- 
cgit v1.2.3