From 8e47474baf06b3884e9173302395dd25fc09eba9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCri=20Valdmann?= <juri.valdmann@qt.io>
Date: Tue, 8 May 2018 15:30:37 +0200
Subject: [PATCH] QJsonDocument: Avoid overflow of string lengths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 src/corelib/serialization/qjson.cpp           |  47 ++++++++----------
 .../invalidBinaryData/{40.json => 40.bjson}   | Bin
 .../json/invalidBinaryData/41.bjson           | Bin 0 -> 32 bytes
 3 files changed, 22 insertions(+), 25 deletions(-)
 rename tests/auto/corelib/serialization/json/invalidBinaryData/{40.json => 40.bjson} (100%)
 create mode 100644 tests/auto/corelib/serialization/json/invalidBinaryData/41.bjson

diff --git a/src/corelib/serialization/qjson.cpp b/src/corelib/serialization/qjson.cpp
index 592f6168dc3..7912b5040c2 100644
--- a/src/corelib/serialization/qjson.cpp
+++ b/src/corelib/serialization/qjson.cpp
@@ -326,38 +326,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::Double:
-        if (latinOrIntValue)
-            break;
-        Q_FALLTHROUGH();
-    case QJsonValue::String:
-    case QJsonValue::Array:
-    case QJsonValue::Object:
-        offset = value;
-        break;
     case QJsonValue::Null:
     case QJsonValue::Bool:
-    default:
-        break;
-    }
-
-    if (offset == -1)
         return true;
-    if (offset + sizeof(uint) > b->tableOffset || offset < (int)sizeof(Base))
+    case QJsonValue::Double:
+        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:
+        return isValidValueOffset(value, b->tableOffset)
+            && static_cast<Object *>(base(b))->isValid(b->tableOffset - value);
+    default:
         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;
+    }
 }
 
 /*!
diff --git a/tests/auto/corelib/serialization/json/invalidBinaryData/40.json b/tests/auto/corelib/serialization/json/invalidBinaryData/40.bjson
similarity index 100%
rename from tests/auto/corelib/serialization/json/invalidBinaryData/40.json
rename to tests/auto/corelib/serialization/json/invalidBinaryData/40.bjson
diff --git a/tests/auto/corelib/serialization/json/invalidBinaryData/41.bjson b/tests/auto/corelib/serialization/json/invalidBinaryData/41.bjson
new file mode 100644
index 0000000000000000000000000000000000000000..0b5940ab95be0d38c56af5bf42b2d01615c36d3b
GIT binary patch
literal 32
gcmXR+$|`1LU|^5{VkRIK0pkDv|JT<61)G5w0AjHQUjP6A

literal 0
HcmV?d00001