From 783d574b932288b61f915b28d5b7b9c5a979f58e Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 6 Mar 2020 13:38:17 -0800 Subject: CBOR support: prevent overflowing QByteArray's max allocation QByteArray doesn't like it. Apply the same protection to QString, which we know uses the same backend but uses elements twice as big. That means it can contain slightly more than half as many elements, but exact half will suffice for our needs. Change-Id: Iaa63461109844e978376fffd15f9d4c7a9137856 Reviewed-by: Edward Welbourne --- src/3rdparty/tinycbor/tests/parser/data.cpp | 37 ++++++++++++++++++----------- src/corelib/serialization/qcborstream.cpp | 35 ++++++++++++++++++++------- src/corelib/serialization/qcborvalue.cpp | 27 ++++++++++++++------- src/corelib/text/qbytearray_p.h | 7 +++--- 4 files changed, 71 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/3rdparty/tinycbor/tests/parser/data.cpp b/src/3rdparty/tinycbor/tests/parser/data.cpp index 0ab0e47be4..3523c32167 100644 --- a/src/3rdparty/tinycbor/tests/parser/data.cpp +++ b/src/3rdparty/tinycbor/tests/parser/data.cpp @@ -338,7 +338,7 @@ void addValidationColumns() QTest::addColumn("expectedError"); } -void addValidationData() +void addValidationData(size_t minInvalid = ~size_t(0)) { // illegal numbers are future extension points QTest::newRow("illegal-number-in-unsigned-1") << raw("\x81\x1c") << 0 << CborErrorIllegalNumber; @@ -488,26 +488,35 @@ void addValidationData() QTest::newRow("map-break-after-value-tag2") << raw("\x81\xbf\0\xd8\x20\xff") << 0 << CborErrorUnexpectedBreak; // check for pointer additions wrapping over the limit of the address space - CborError tooLargeOn32bit = (sizeof(void *) == 4) ? CborErrorDataTooLarge : CborErrorUnexpectedEOF; + auto wraparoundError = [minInvalid](uint64_t encodedSize) { + if (encodedSize > minInvalid) + return CborErrorDataTooLarge; + return CborErrorUnexpectedEOF; + }; + constexpr uint64_t FourGB = UINT32_MAX + UINT64_C(1); // on 32-bit systems, this is a -1 - QTest::newRow("bytearray-wraparound1") << raw("\x81\x5a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF; - QTest::newRow("string-wraparound1") << raw("\x81\x7a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF; + QTest::newRow("bytearray-wraparound1") << raw("\x81\x5a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX); + QTest::newRow("string-wraparound1") << raw("\x81\x7a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX); // on 32-bit systems, a 4GB addition could be dropped - QTest::newRow("bytearray-wraparound2") << raw("\x81\x5b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit; - QTest::newRow("string-wraparound2") << raw("\x81\x7b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit; + QTest::newRow("bytearray-wraparound2") << raw("\x81\x5b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB); + QTest::newRow("string-wraparound2") << raw("\x81\x7b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB); // on 64-bit systems, this could be a -1 - QTest::newRow("bytearray-wraparound3") << raw("\x81\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit; - QTest::newRow("string-wraparound3") << raw("\x81\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit; + QTest::newRow("bytearray-wraparound3") << raw("\x81\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 + << wraparoundError(UINT64_MAX); + QTest::newRow("string-wraparound3") << raw("\x81\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 + << wraparoundError(UINT64_MAX); // ditto on chunks - QTest::newRow("bytearray-chunk-wraparound1") << raw("\x81\x5f\x5a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF; - QTest::newRow("string-chunk-wraparound1") << raw("\x81\x7f\x7a\xff\xff\xff\xff") << 0 << CborErrorUnexpectedEOF; + QTest::newRow("bytearray-chunk-wraparound1") << raw("\x81\x5f\x5a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX); + QTest::newRow("string-chunk-wraparound1") << raw("\x81\x7f\x7a\xff\xff\xff\xff") << 0 << wraparoundError(UINT32_MAX); // on 32-bit systems, a 4GB addition could be dropped - QTest::newRow("bytearray-chunk-wraparound2") << raw("\x81\x5f\x5b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit; - QTest::newRow("string-chunk-wraparound2") << raw("\x81\x7f\x7b\0\0\0\1\0\0\0\0") << 0 << tooLargeOn32bit; + QTest::newRow("bytearray-chunk-wraparound2") << raw("\x81\x5f\x5b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB); + QTest::newRow("string-chunk-wraparound2") << raw("\x81\x7f\x7b\0\0\0\1\0\0\0\0") << 0 << wraparoundError(FourGB); // on 64-bit systems, this could be a -1 - QTest::newRow("bytearray-chunk-wraparound3") << raw("\x81\x5f\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit; - QTest::newRow("string-chunk-wraparound3") << raw("\x81\x7f\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 << tooLargeOn32bit; + QTest::newRow("bytearray-chunk-wraparound3") << raw("\x81\x5f\x5b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 + << wraparoundError(UINT64_MAX); + QTest::newRow("string-chunk-wraparound3") << raw("\x81\x7f\x7b\xff\xff\xff\xff\xff\xff\xff\xff") << 0 + << wraparoundError(UINT64_MAX); QTest::newRow("eof-after-array") << raw("\x81") << 0 << CborErrorUnexpectedEOF; QTest::newRow("eof-after-array2") << raw("\x81\x78\x20") << 0 << CborErrorUnexpectedEOF; diff --git a/src/corelib/serialization/qcborstream.cpp b/src/corelib/serialization/qcborstream.cpp index c598eee213..22748e49e1 100644 --- a/src/corelib/serialization/qcborstream.cpp +++ b/src/corelib/serialization/qcborstream.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2018 Intel Corporation. +** Copyright (C) 2020 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -39,6 +39,7 @@ #include "qcborstream.h" +#include #include #include #include @@ -2282,6 +2283,10 @@ bool QCborStreamReader::next(int maxRecursion) } else if (isString() || isByteArray()) { auto r = _readByteArray_helper(); while (r.status == Ok) { + if (isString() && r.data.size() > MaxStringSize) { + d->handleError(CborErrorDataTooLarge); + break; + } if (isString() && !QUtf8::isValidUtf8(r.data, r.data.size()).isValidUtf8) { d->handleError(CborErrorInvalidUtf8TextString); break; @@ -2564,15 +2569,23 @@ QCborStreamReader::StringResult QCborStreamReader::_readString_helper() result.status = r.status; if (r.status == Ok) { - QTextCodec::ConverterState cs; - result.data = QUtf8::convertToUnicode(r.data, r.data.size(), &cs); - if (cs.invalidChars == 0 && cs.remainingChars == 0) - return result; + // See QUtf8::convertToUnicode() a detailed explanation of why this + // conversion uses the same number of words or less. + CborError err = CborNoError; + if (r.data.size() > MaxStringSize) { + err = CborErrorDataTooLarge; + } else { + QTextCodec::ConverterState cs; + result.data = QUtf8::convertToUnicode(r.data, r.data.size(), &cs); + if (cs.invalidChars != 0 || cs.remainingChars != 0) + err = CborErrorInvalidUtf8TextString; + } - d->handleError(CborErrorInvalidUtf8TextString); - result.data.clear(); - result.status = Error; - return result; + if (err) { + d->handleError(err); + result.data.clear(); + result.status = Error; + } } return result; } @@ -2600,6 +2613,10 @@ QCborStreamReader::StringResult QCborStreamReader::_readByteArray_he qsizetype len = _currentStringChunkSize(); if (len < 0) return result; + if (len > MaxByteArraySize) { + d->handleError(CborErrorDataTooLarge); + return result; + } result.data.resize(len); auto r = readStringChunk(result.data.data(), len); diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index f4ceba3836..65abf8dba8 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2018 Intel Corporation. +** Copyright (C) 2020 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -46,6 +46,7 @@ #include #include +#include #include #include @@ -1534,6 +1535,8 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) // and calculate the final size if (add_overflow(offset, increment, &newSize)) return -1; + if (newSize > MaxByteArraySize) + return -1; // since usedData <= data.size(), this can't overflow usedData += increment; @@ -1608,13 +1611,6 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) setErrorInReader(reader, { QCborError::DataTooLarge }); } - if (r.status == QCborStreamReader::Error) { - // There can only be errors if there was data to be read. - Q_ASSERT(e.flags & Element::HasByteData); - data.truncate(e.value); - return; - } - // update size if (e.flags & Element::HasByteData) { auto b = new (dataPtr() + e.value) ByteData; @@ -1626,6 +1622,21 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) Q_ASSERT(e.type == QCborValue::String); e.flags |= Element::StringIsAscii; } + + // check that this UTF-8 text string can be loaded onto a QString + if (e.type == QCborValue::String) { + if (Q_UNLIKELY(b->len > MaxStringSize)) { + setErrorInReader(reader, { QCborError::DataTooLarge }); + r.status = QCborStreamReader::Error; + } + } + } + + if (r.status == QCborStreamReader::Error) { + // There can only be errors if there was data to be read. + Q_ASSERT(e.flags & Element::HasByteData); + data.truncate(e.value); + return; } elements.append(e); diff --git a/src/corelib/text/qbytearray_p.h b/src/corelib/text/qbytearray_p.h index 3c6257f786..ffec6dca22 100644 --- a/src/corelib/text/qbytearray_p.h +++ b/src/corelib/text/qbytearray_p.h @@ -56,10 +56,9 @@ QT_BEGIN_NAMESPACE -enum { - // Define as enum to force inlining. Don't expose MaxAllocSize in a public header. - MaxByteArraySize = MaxAllocSize - sizeof(std::remove_pointer::type) -}; +// -1 because of the terminating NUL +constexpr qsizetype MaxByteArraySize = MaxAllocSize - sizeof(std::remove_pointer::type) - 1; +constexpr qsizetype MaxStringSize = (MaxAllocSize - sizeof(std::remove_pointer::type)) / 2 - 1; QT_END_NAMESPACE -- cgit v1.2.3 From 2e0c29a4bbe2b3ae427137e65f179b0550dcf169 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Tue, 11 Feb 2020 13:58:04 +0100 Subject: itemviews: Use the start of the current selection when getting the range When doing a shift-select while moving the mouse then the start point should be based on the start of the current selection and not the pressed position. If there is no current selection start index, then we can safely depend on pressed position as this will be the previous index pressed on. This resolves an issue introduced by e02293a76d21e7077f1952d4ed8af6c6d1970190 when fixing QTBUG-78797 Fixes: QTBUG-81542 Change-Id: Ia66c42b220452fdcbc8cfccc05dbc8a3911c3f5e Reviewed-by: Richard Moe Gustavsen --- src/widgets/itemviews/qabstractitemview.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index f879af6ad2..73604cb8f4 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -1844,10 +1844,16 @@ void QAbstractItemView::mouseMoveEvent(QMouseEvent *event) || edit(index, NoEditTriggers, event)) return; - if (d->selectionMode != SingleSelection) - topLeft = d->pressedPosition - d->offset(); - else + if (d->selectionMode != SingleSelection) { + // Use the current selection start index if it is valid as this will be based on the + // start of the selection and not the last item being pressed which can be different + // when in extended selection + topLeft = d->currentSelectionStartIndex.isValid() + ? visualRect(d->currentSelectionStartIndex).center() + : d->pressedPosition - d->offset(); + } else { topLeft = bottomRight; + } d->checkMouseMove(index); -- cgit v1.2.3 From 22fe29303869ccf31af25cd4eeeaad768c3f647b Mon Sep 17 00:00:00 2001 From: Liang Qi Date: Fri, 27 Mar 2020 14:17:39 +0100 Subject: testlib: add QAbstractItemModelTester::verify() This amends b3e4be2d8b9debf217657436139da0152f6f8797. When building testlib with QtGui linked:(use "QT = core-private gui" in src/testlib/testlib.pro) Undefined symbols for architecture x86_64: "QAbstractItemModelTester::verify(bool, char const*, char const*, char const*, int)", referenced from: QTestPrivate::testDataGuiRoles(QAbstractItemModelTester*) in qabstractitemmodeltester.o Change-Id: Ideb10ddd6717fed8d9f91f75bbfc9d5a22104730 Reviewed-by: Giuseppe D'Angelo --- src/testlib/qabstractitemmodeltester.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/testlib/qabstractitemmodeltester.cpp b/src/testlib/qabstractitemmodeltester.cpp index 859966c0e3..f651fd95be 100644 --- a/src/testlib/qabstractitemmodeltester.cpp +++ b/src/testlib/qabstractitemmodeltester.cpp @@ -288,6 +288,12 @@ QAbstractItemModelTester::FailureReportingMode QAbstractItemModelTester::failure return d->failureReportingMode; } +bool QAbstractItemModelTester::verify(bool statement, const char *statementStr, const char *description, const char *file, int line) +{ + Q_D(QAbstractItemModelTester); + return d->verify(statement, statementStr, description, file, line); +} + QAbstractItemModelTesterPrivate::QAbstractItemModelTesterPrivate(QAbstractItemModel *model, QAbstractItemModelTester::FailureReportingMode failureReportingMode) : model(model), failureReportingMode(failureReportingMode), -- cgit v1.2.3 From d934fd7f54eae24ea3f719890e2c4dbbc445049d Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 6 Mar 2020 17:17:40 +0100 Subject: Send MouseMove events without buttons if the press closed the popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With nested popup widgets, pressing a mouse button on the lower popup will close the active popup. MouseMove events that are generated before the button is released again should not have that button included, as it is likely to result in incorrect state handling in the widget. This change removes all buttons from the MouseMove event, which is the second best option. This is mostly consistent with the behavior when closing a popup and no other popup remains. The widget underneath will get MouseMove events without the respective button included. This change doesn't include a fix for the final release event, which should ideally also not be delivered to the remaining popup, as it never got a corresponding press event. Qt has already reset the states in which it stores which widget received the press event at the time the release is generated, such as qt_button_down and qt_popup_down. So we can't separate a release grabbed by a newly opened popup (which we want) from a release to the popup that became active after closing (which we don't want). However, widgets can more easily work around this issue, and the risk of breaking things by changing the code further becomes too high. Change-Id: I603bbdbc7e7355952d96ab77c5e2d2f1e6f94987 Fixes: QTBUG-82538 Reviewed-by: Tor Arne Vestbø Reviewed-by: Gatis Paeglis --- src/widgets/kernel/qwidgetwindow.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp index 596343c52f..7b36699d5c 100644 --- a/src/widgets/kernel/qwidgetwindow.cpp +++ b/src/widgets/kernel/qwidgetwindow.cpp @@ -505,7 +505,7 @@ void QWidgetWindow::handleMouseEvent(QMouseEvent *event) QGuiApplicationPrivate::platformTheme()->themeHint(QPlatformTheme::ContextMenuOnMouseRelease).toBool() ? QEvent::MouseButtonRelease : QEvent::MouseButtonPress; if (QApplicationPrivate::inPopupMode()) { - QWidget *activePopupWidget = QApplication::activePopupWidget(); + QPointer activePopupWidget = QApplication::activePopupWidget(); QPoint mapped = event->pos(); if (activePopupWidget != m_widget) mapped = activePopupWidget->mapFromGlobal(event->globalPos()); @@ -565,9 +565,11 @@ void QWidgetWindow::handleMouseEvent(QMouseEvent *event) #endif if ((event->type() != QEvent::MouseButtonPress) || !(event->flags().testFlag(Qt::MouseEventCreatedDoubleClick))) { - + // if the widget that was pressed is gone, then deliver move events without buttons + const auto buttons = event->type() == QEvent::MouseMove && qt_button_down == nullptr + ? Qt::NoButton : event->buttons(); QMouseEvent e(event->type(), widgetPos, event->windowPos(), event->screenPos(), - event->button(), event->buttons(), event->modifiers(), event->source()); + event->button(), buttons, event->modifiers(), event->source()); e.setTimestamp(event->timestamp()); QApplicationPrivate::sendMouseEvent(receiver, &e, receiver, receiver->window(), &qt_button_down, qt_last_mouse_receiver); qt_last_mouse_receiver = receiver; -- cgit v1.2.3