diff options
author | Jüri Valdmann <juri.valdmann@qt.io> | 2018-05-08 15:30:37 +0200 |
---|---|---|
committer | Jüri Valdmann <juri.valdmann@qt.io> | 2018-05-16 08:54:57 +0000 |
commit | f24bd1b8183ec9cc4e239dc560072d3896ec61a1 (patch) | |
tree | 4bf2114a7afe61d6d185e6d418f950e567942fba /src/corelib/json | |
parent | 60a7d578c8add335900d4d1006b3b2c49f141873 (diff) |
QJsonDocument: Avoid overflow of string lengths
The added test case contains the binary JSON equivalent of
["ž"]
with the modification that the string's length has been set to INT_MAX. In
Value::usedStorage this length is used through the pointer d like so
s = sizeof(int) + sizeof(ushort) * qFromLittleEndian(*(int *)d);
Because 2 * INT_MAX is UINT_MAX-1, the expression as a whole evaluates to 2,
which is considered a valid storage size. However, when converting this binary
JSON into ordinary JSON we will attempt to construct a QString of length
INT_MAX.
Fixed by using String::isValid instead of Value::usedStorage. This method
already takes care to avoid the overflow problem. Additionally, I've tried in
this patch to clarify the behavior of Value::isValid a bit by writing it in a
style that is hopefully more amenable to structural induction.
Finally, the test case added in my previous patch had the wrong file extension
and is renamed in this one.
Task-number: QTBUG-61969
Change-Id: I45d891f2467a71d8d105822ef7eb1a73c3efa67a
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 8e47474baf06b3884e9173302395dd25fc09eba9)
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Diffstat (limited to 'src/corelib/json')
-rw-r--r-- | src/corelib/json/qjson.cpp | 43 |
1 files changed, 20 insertions, 23 deletions
diff --git a/src/corelib/json/qjson.cpp b/src/corelib/json/qjson.cpp index 944c7695c6..31260ef5fd 100644 --- a/src/corelib/json/qjson.cpp +++ b/src/corelib/json/qjson.cpp @@ -333,38 +333,35 @@ int Value::usedStorage(const Base *b) const return alignedSize(s); } +inline bool isValidValueOffset(uint offset, uint tableOffset) +{ + return offset >= sizeof(Base) + && offset + sizeof(uint) <= tableOffset; +} + bool Value::isValid(const Base *b) const { - int offset = -1; switch (type) { + case QJsonValue::Null: + case QJsonValue::Bool: + return true; case QJsonValue::Double: - if (latinOrIntValue) - break; - Q_FALLTHROUGH(); + return latinOrIntValue || isValidValueOffset(value, b->tableOffset); case QJsonValue::String: + if (!isValidValueOffset(value, b->tableOffset)) + return false; + if (latinOrIntValue) + return asLatin1String(b).isValid(b->tableOffset - value); + return asString(b).isValid(b->tableOffset - value); case QJsonValue::Array: + return isValidValueOffset(value, b->tableOffset) + && static_cast<Array *>(base(b))->isValid(b->tableOffset - value); case QJsonValue::Object: - offset = value; - break; - case QJsonValue::Null: - case QJsonValue::Bool: + return isValidValueOffset(value, b->tableOffset) + && static_cast<Object *>(base(b))->isValid(b->tableOffset - value); default: - break; - } - - if (offset == -1) - return true; - if (offset + sizeof(uint) > b->tableOffset || offset < (int)sizeof(Base)) - return false; - - int s = usedStorage(b); - if (s < 0 || s > (int)b->tableOffset - offset) return false; - if (type == QJsonValue::Array) - return static_cast<Array *>(base(b))->isValid(s); - if (type == QJsonValue::Object) - return static_cast<Object *>(base(b))->isValid(s); - return true; + } } /*! |