summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorUlf Hermann <ulf.hermann@qt.io>2019-03-26 09:40:03 +0100
committerUlf Hermann <ulf.hermann@qt.io>2019-03-27 09:23:14 +0000
commit1e18f2c4a647923fc66a3e3204fcccd88a2960a6 (patch)
treebdedb8120674da3c7abc4eea19f197ddca2ee5d4
parent60f766f5c68fc33322c6d095d81b1856828b2b0b (diff)
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>
-rw-r--r--src/qml/jsruntime/qv4internalclass.cpp28
-rw-r--r--src/qml/jsruntime/qv4internalclass_p.h8
-rw-r--r--src/qml/jsruntime/qv4memberdata.cpp21
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;