From fdea55cb9832a194b5ec1262e216f12ae644ba6b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 15 Apr 2020 18:52:49 -0300 Subject: QCborValue: fix double-accounting of the usedData when decoding strings We can only update usedData at the end, after we've decoded all chunks. The update inside the lambda was double-accounting for the first chunk, which could lead to signed integer overflows in usedData. Not unit-testable since the usedData value is not visible in the API. Change-Id: Ibdc95e9af7bd456a94ecfffd16061cc955208859 Reviewed-by: Ulf Hermann Reviewed-by: Edward Welbourne --- src/corelib/serialization/qcborvalue.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/corelib/serialization') diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index c45a09ad99..90b45fb853 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -1561,8 +1561,6 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) if (newSize > MaxByteArraySize) return -1; - // since usedData <= data.size(), this can't overflow - usedData += increment; data.resize(newSize); return offset; }; -- cgit v1.2.3 From c197615bd99ec76ebb3ca9aff959826b4e7ff43b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 15 Apr 2020 19:10:01 -0300 Subject: QCborValue: don't update internal states if decoding a string failed Change-Id: Ibdc95e9af7bd456a94ecfffd16061db982ad3fa7 Reviewed-by: Ulf Hermann Reviewed-by: Edward Welbourne --- src/corelib/serialization/qcborvalue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/serialization') diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index 90b45fb853..a3729b4ef9 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -1633,7 +1633,7 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) } // update size - if (e.flags & Element::HasByteData) { + if (r.status == QCborStreamReader::EndOfString && e.flags & Element::HasByteData) { auto b = new (dataPtr() + e.value) ByteData; b->len = data.size() - e.value - int(sizeof(*b)); usedData += b->len; -- cgit v1.2.3 From 9e83d268d6e0bd492fafad823a47cef57b7916c5 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Wed, 22 Apr 2020 20:06:16 +0200 Subject: Fix data corruption regression in QJsonObject::erase() The internal removeAt(index) method was implemented as taking cbor indexes directly, in contrast to the other ...At(index) methods. Fixes: QTBUG-83695 Change-Id: I16597eb6db1cf71e1585c041caa81bf8f7a75303 Reviewed-by: Thiago Macieira --- src/corelib/serialization/qjsonobject.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/corelib/serialization') diff --git a/src/corelib/serialization/qjsonobject.cpp b/src/corelib/serialization/qjsonobject.cpp index 850e878571..36429662fc 100644 --- a/src/corelib/serialization/qjsonobject.cpp +++ b/src/corelib/serialization/qjsonobject.cpp @@ -577,7 +577,7 @@ void QJsonObject::removeImpl(T key) if (!keyExists) return; - removeAt(index); + removeAt(index / 2); } #if QT_STRINGVIEW_LEVEL < 2 @@ -629,7 +629,7 @@ QJsonValue QJsonObject::takeImpl(T key) return QJsonValue(QJsonValue::Undefined); const QJsonValue v = QJsonPrivate::Value::fromTrustedCbor(o->extractAt(index + 1)); - removeAt(index); + removeAt(index / 2); return v; } @@ -1486,8 +1486,8 @@ void QJsonObject::setValueAt(int i, const QJsonValue &val) void QJsonObject::removeAt(int index) { detach2(); - o->removeAt(index + 1); - o->removeAt(index); + o->removeAt(2 * index + 1); + o->removeAt(2 * index); } uint qHash(const QJsonObject &object, uint seed) -- cgit v1.2.3 From 52a2505672cbb1ca8b5b32f7bc1259485c65483b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 15 Apr 2020 13:58:33 -0300 Subject: QCborValue: avoid signed integer oveflows when decoding time_t QDateTime::fromSecsSinceEpoch() multiplies by 1000 but does not check for overflow. That means we must do so in QCborValue validation. We can't use mul_overflow on 32-bit platforms, so we do a compare- and-branch there. For 64-bit platforms, we prefer to do the multiplication with checked overflow, as the common case is that it will not overflow and we'll need the multiplication anyway. Change-Id: Ibdc95e9af7bd456a94ecfffd16060cba6f1c86b8 Reviewed-by: Edward Welbourne --- src/corelib/serialization/qcborvalue.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'src/corelib/serialization') diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index a3729b4ef9..f5cccf1be1 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -788,10 +788,25 @@ static QCborValue::Type convertToExtendedType(QCborContainerPrivate *d) // The data is supposed to be US-ASCII. If it isn't (contains UTF-8), // QDateTime::fromString will fail anyway. dt = QDateTime::fromString(b->asLatin1(), Qt::ISODateWithMs); - } else if (tag == qint64(QCborKnownTags::UnixTime_t) && e.type == QCborValue::Integer) { - dt = QDateTime::fromSecsSinceEpoch(e.value, Qt::UTC); - } else if (tag == qint64(QCborKnownTags::UnixTime_t) && e.type == QCborValue::Double) { - dt = QDateTime::fromMSecsSinceEpoch(qint64(e.fpvalue() * 1000), Qt::UTC); + } else if (tag == qint64(QCborKnownTags::UnixTime_t)) { + qint64 msecs; + bool ok = false; + if (e.type == QCborValue::Integer) { +#if QT_POINTER_SIZE == 8 + // we don't have a fast 64-bit mul_overflow implementation on + // 32-bit architectures. + ok = !mul_overflow(e.value, qint64(1000), &msecs); +#else + static const qint64 Limit = std::numeric_limits::max() / 1000; + ok = (e.value > -Limit && e.value < Limit); + if (ok) + msecs = e.value * 1000; +#endif + } else if (e.type == QCborValue::Double) { + ok = convertDoubleTo(round(e.fpvalue() * 1000), &msecs); + } + if (ok) + dt = QDateTime::fromMSecsSinceEpoch(msecs, Qt::UTC); } if (dt.isValid()) { QByteArray text = dt.toString(Qt::ISODateWithMs).toLatin1(); -- cgit v1.2.3 From 2a53017df488b3cedbfbf5a56bf105b9fc5392fa Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 15 Apr 2020 14:00:05 -0300 Subject: QCborValue: add an extra check against producing invalid ISO dates By QCborValue design, we store the textual representation in ISO format, equivalent of CBOR tag 0, which isn't allowed to have negative years or beyond year 10000. Change-Id: Ibdc95e9af7bd456a94ecfffd16060ccff359c296 Reviewed-by: Ulf Hermann --- src/corelib/serialization/qcborvalue.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src/corelib/serialization') diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index f5cccf1be1..30bfa367ed 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -810,10 +810,12 @@ static QCborValue::Type convertToExtendedType(QCborContainerPrivate *d) } if (dt.isValid()) { QByteArray text = dt.toString(Qt::ISODateWithMs).toLatin1(); - replaceByteData(text, text.size(), Element::StringIsAscii); - e.type = QCborValue::String; - d->elements[0].value = qint64(QCborKnownTags::DateTimeString); - return QCborValue::DateTime; + if (!text.isEmpty()) { + replaceByteData(text, text.size(), Element::StringIsAscii); + e.type = QCborValue::String; + d->elements[0].value = qint64(QCborKnownTags::DateTimeString); + return QCborValue::DateTime; + } } break; } -- cgit v1.2.3 From 821e71fded090d815b5cd396057ac9823874fe1f Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 15 Apr 2020 15:16:06 -0300 Subject: QCborValue: check parsing of invalid URL QUrl will reject invalid URLs for us, so we don't get normalization. The original junk should be retrievable, of course. Change-Id: Ibdc95e9af7bd456a94ecfffd160610f5b2c8e1a2 Reviewed-by: Ulf Hermann Reviewed-by: Edward Welbourne --- src/corelib/serialization/qcborvalue.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/corelib/serialization') diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index 30bfa367ed..3bca15d562 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -827,9 +827,11 @@ static QCborValue::Type convertToExtendedType(QCborContainerPrivate *d) // normalize to a short (decoded) form, so as to save space QUrl url(e.flags & Element::StringIsUtf16 ? b->asQStringRaw() : - b->toUtf8String()); - QByteArray encoded = url.toString(QUrl::DecodeReserved).toUtf8(); - replaceByteData(encoded, encoded.size(), {}); + b->toUtf8String(), QUrl::StrictMode); + if (url.isValid()) { + QByteArray encoded = url.toString(QUrl::DecodeReserved).toUtf8(); + replaceByteData(encoded, encoded.size(), {}); + } } return QCborValue::Url; } -- cgit v1.2.3