From 2efeaab1c6387ba39473aa91f1fde206e731ebe5 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 1 Apr 2025 07:15:12 +0200 Subject: [PATCH] QComboBox: fix UB (signed overflow) in Private::recomputeSizeHint() The minimumContentsLength() is user-settable, and tst_QComboBox actually sets it to INT_MAX as some point. As such, the implementation needs to defend itself from UB, either by limiting what values it accepts as minimumContentsLength() or by making sure it doesn't cause signed overflow in calculations involving this user-controlled variable. The former doesn't really work as long as the test checks that INT_MAX is accepted as-is, so we need to do the latter. Do the calculation in qint64, and use the result only when it doesn't exceeed good old QWIDGETSIZE_MAX. Amends the start of the public history. Pick-to: 6.8 6.5 5.15 Change-Id: I8aac7bda593095638c9c09db3b70b4c84e698419 Reviewed-by: Axel Spoerl (cherry picked from commit 00fdf9e432c78e7d7f1a491f7bc1f375fcbde8da) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/widgets/qcombobox.cpp | 15 +++++++++++++-- .../widgets/widgets/qcombobox/tst_qcombobox.cpp | 4 ++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/widgets/widgets/qcombobox.cpp b/src/widgets/widgets/qcombobox.cpp index 00c23856280..e11f20101e9 100644 --- a/src/widgets/widgets/qcombobox.cpp +++ b/src/widgets/widgets/qcombobox.cpp @@ -384,8 +384,19 @@ QSize QComboBoxPrivate::recomputeSizeHint(QSize &sh) const for (int i = 0; i < count && !hasIcon; ++i) hasIcon = !q->itemIcon(i).isNull(); } - if (minimumContentsLength > 0) - sh.setWidth(qMax(sh.width(), minimumContentsLength * fm.horizontalAdvance(u'X') + (hasIcon ? iconSize.width() + 4 : 0))); + if (minimumContentsLength > 0) { + auto r = qint64{minimumContentsLength} * fm.horizontalAdvance(u'X'); + if (hasIcon) + r += iconSize.width() + 4; + if (r <= QWIDGETSIZE_MAX) { + sh.setWidth(qMax(sh.width(), int(r))); + } else { + qWarning("QComboBox: cannot take minimumContentsLength %d into account for sizeHint(), " + "since it causes the widget to be wider than QWIDGETSIZE_MAX. " + "Consider setting it to a less extreme value.", + minimumContentsLength); + } + } if (!placeholderText.isEmpty()) sh.setWidth(qMax(sh.width(), fm.boundingRect(placeholderText).width())); diff --git a/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp b/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp index e77fd804f21..bb05a94386b 100644 --- a/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp +++ b/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp @@ -297,6 +297,10 @@ void tst_QComboBox::getSetCheck() QCOMPARE(100, obj1.minimumContentsLength()); obj1.setMinimumContentsLength(INT_MIN); QCOMPARE(100, obj1.minimumContentsLength()); // Cannot be set to something negative => old value + QTest::ignoreMessage(QtWarningMsg, // not necessarily here, but upon first sizeHint() call + "QComboBox: cannot take minimumContentsLength 2147483647 into account for sizeHint(), " + "since it causes the widget to be wider than QWIDGETSIZE_MAX. " + "Consider setting it to a less extreme value."); obj1.setMinimumContentsLength(INT_MAX); QCOMPARE(INT_MAX, obj1.minimumContentsLength());