Skip to content

Commit 8e47474

Browse files
committed
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 <[email protected]>
1 parent bff2101 commit 8e47474

File tree

3 files changed

+20
-23
lines changed

3 files changed

+20
-23
lines changed

src/corelib/serialization/qjson.cpp

+20-23
Original file line numberDiff line numberDiff line change
@@ -326,38 +326,35 @@ int Value::usedStorage(const Base *b) const
326326
return alignedSize(s);
327327
}
328328

329+
inline bool isValidValueOffset(uint offset, uint tableOffset)
330+
{
331+
return offset >= sizeof(Base)
332+
&& offset + sizeof(uint) <= tableOffset;
333+
}
334+
329335
bool Value::isValid(const Base *b) const
330336
{
331-
int offset = -1;
332337
switch (type) {
338+
case QJsonValue::Null:
339+
case QJsonValue::Bool:
340+
return true;
333341
case QJsonValue::Double:
334-
if (latinOrIntValue)
335-
break;
336-
Q_FALLTHROUGH();
342+
return latinOrIntValue || isValidValueOffset(value, b->tableOffset);
337343
case QJsonValue::String:
344+
if (!isValidValueOffset(value, b->tableOffset))
345+
return false;
346+
if (latinOrIntValue)
347+
return asLatin1String(b).isValid(b->tableOffset - value);
348+
return asString(b).isValid(b->tableOffset - value);
338349
case QJsonValue::Array:
350+
return isValidValueOffset(value, b->tableOffset)
351+
&& static_cast<Array *>(base(b))->isValid(b->tableOffset - value);
339352
case QJsonValue::Object:
340-
offset = value;
341-
break;
342-
case QJsonValue::Null:
343-
case QJsonValue::Bool:
353+
return isValidValueOffset(value, b->tableOffset)
354+
&& static_cast<Object *>(base(b))->isValid(b->tableOffset - value);
344355
default:
345-
break;
346-
}
347-
348-
if (offset == -1)
349-
return true;
350-
if (offset + sizeof(uint) > b->tableOffset || offset < (int)sizeof(Base))
351-
return false;
352-
353-
int s = usedStorage(b);
354-
if (s < 0 || s > (int)b->tableOffset - offset)
355356
return false;
356-
if (type == QJsonValue::Array)
357-
return static_cast<Array *>(base(b))->isValid(s);
358-
if (type == QJsonValue::Object)
359-
return static_cast<Object *>(base(b))->isValid(s);
360-
return true;
357+
}
361358
}
362359

363360
/*!
Binary file not shown.

0 commit comments

Comments
 (0)