From 961b8f51a2e8198fce12e8784b1edae6b3f6f67b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCri=20Valdmann?= Date: Thu, 3 May 2018 13:25:06 +0200 Subject: QJsonDocument::fromRawData: Fix out-of-bounds access This method takes a pointer+size pair, but begins reading through the pointer without first checking the size parameter. Fixed by checking the size parameter. A new test case is added with an empty binary json file. Although the test does not fail under normal conditions, the problem can be detected using valgrind or AddressSanitizer. Task-number: QTBUG-61969 Change-Id: Ie91cc9a56dbc3c676472c614d4e633d7721b8481 Reviewed-by: Lars Knoll Reviewed-by: Thiago Macieira (cherry picked from commit d3935cbd71171e1d8f3742cc3235ca0c38313ec8) --- src/corelib/json/qjson_p.h | 2 +- src/corelib/json/qjsondocument.cpp | 3 +++ tests/auto/corelib/json/invalidBinaryData/38.bjson | 0 tests/auto/corelib/json/tst_qtjson.cpp | 1 + 4 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 tests/auto/corelib/json/invalidBinaryData/38.bjson diff --git a/src/corelib/json/qjson_p.h b/src/corelib/json/qjson_p.h index c012ec2662..acedd427d6 100644 --- a/src/corelib/json/qjson_p.h +++ b/src/corelib/json/qjson_p.h @@ -578,7 +578,7 @@ static inline void copyString(char *dest, const QString &str, bool compress) /* - Base is the base class for both Object and Array. Both classe work more or less the same way. + Base is the base class for both Object and Array. Both classes work more or less the same way. The class starts with a header (defined by the struct below), then followed by data (the data for values in the Array case and Entry's (see below) for objects. diff --git a/src/corelib/json/qjsondocument.cpp b/src/corelib/json/qjsondocument.cpp index ed454d5442..58cd01588f 100644 --- a/src/corelib/json/qjsondocument.cpp +++ b/src/corelib/json/qjsondocument.cpp @@ -188,6 +188,9 @@ QJsonDocument QJsonDocument::fromRawData(const char *data, int size, DataValidat return QJsonDocument(); } + if (size < (int)(sizeof(QJsonPrivate::Header) + sizeof(QJsonPrivate::Base))) + return QJsonDocument(); + QJsonPrivate::Data *d = new QJsonPrivate::Data((char *)data, size); d->ownsData = false; diff --git a/tests/auto/corelib/json/invalidBinaryData/38.bjson b/tests/auto/corelib/json/invalidBinaryData/38.bjson new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/auto/corelib/json/tst_qtjson.cpp b/tests/auto/corelib/json/tst_qtjson.cpp index b215364f0e..275e6bfeb1 100644 --- a/tests/auto/corelib/json/tst_qtjson.cpp +++ b/tests/auto/corelib/json/tst_qtjson.cpp @@ -1810,6 +1810,7 @@ void tst_QtJson::invalidBinaryData() QFile file(files.at(i).filePath()); file.open(QIODevice::ReadOnly); QByteArray bytes = file.readAll(); + bytes.squeeze(); QJsonDocument document = QJsonDocument::fromRawData(bytes.constData(), bytes.size()); QVERIFY(document.isNull()); } -- cgit v1.2.3