QBindable: Fix out-of-bound access in non-bindable property support code

QObjectPrivate::getPropertyAdaptorSlotObject called
connectionsForSignal.
Calling this function is only safe after it has been ensured beforehand that the vector has size > signalIndex. As getPropertyAdaptorSlotObject
is not supposed to modify the vector, it does not resize the vector and it could consequently end up with an out-of-bounds read.

To avoid that issue, we instead first check if the vector can
potentially contain an entry for the signal. If not, we simply return
nullptr, and avoid the call to connectionsForSignal.

The issue and its fix can be verified by running the modified
tst_qproperty test with ASAN enabled. The test is modified in the
following way:
- We first create a signal connection to a dummy slot. Otherwise,
  connections.loadRelaxed() would return a nullptr, and the problematic
  code would never be reached.
- We add enough signals to ensure that the fooChanged signal will
  actually be out of reach (which means >= 8 signals, as the initial
  capacity of the vector is 8)
Running the test without ASAN will most likely not result in a failure,
as then the out-of-bounds read will simply read garbage, and the most
likely result is that the cast below will fail.

Change-Id: I18a3c4f52769c2b6491a685abb84f6fcfb44e4d8
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
(cherry picked from commit 96e1381a0a61a5b26f7147516f107765dbfc083e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Fabian Kosmale 2023-06-19 16:24:51 +02:00 committed by Qt Cherry-pick Bot
parent bdeb45c2e2
commit 295ecd9b87
2 changed files with 14 additions and 2 deletions

View File

@ -5384,7 +5384,9 @@ QObjectPrivate::getPropertyAdaptorSlotObject(const QMetaProperty &property)
Q_Q(QObject);
const QMetaObject *metaObject = q->metaObject();
int signal_index = methodIndexToSignalIndex(&metaObject, property.notifySignalIndex());
auto connectionList = conns->connectionsForSignal(signal_index);
if (signal_index >= conns->signalVectorCount())
return nullptr;
const auto connectionList = conns->connectionsForSignal(signal_index);
for (auto c = connectionList.first.loadRelaxed(); c;
c = c->nextConnectionList.loadRelaxed()) {
if (c->isSlotObject) {

View File

@ -1711,6 +1711,14 @@ class PropertyAdaptorTester : public QObject
Q_PROPERTY(int foo1 READ foo WRITE setFoo)
signals:
void dummySignal1();
void dummySignal2();
void dummySignal3();
void dummySignal4();
void dummySignal5();
void dummySignal6();
void dummySignal7();
void dummySignal8();
void fooChanged(int newFoo);
public slots:
@ -1739,9 +1747,11 @@ void tst_QProperty::propertyAdaptorBinding()
// Check binding of non BINDABLE property
PropertyAdaptorTester object;
// set up a dummy connection (needed to verify that the QBindable avoids an out-of-bounds read)
QObject::connect(&object, &PropertyAdaptorTester::dummySignal1, [](){});
QBindable<int> binding(&object, "foo");
QObject::connect(&object, &PropertyAdaptorTester::fooChanged, &object,
&PropertyAdaptorTester::fooHasChanged);
QBindable<int> binding(&object, "foo");
binding.setBinding([&]() { return source + 1; });
QCOMPARE(object.foo(), 6);
QCOMPARE(object.fooChangedCount, 1);