Fix out-of-bounds write

Change ac210c7 introduced the requirement that all input devices with Scroll capability must have a QXcbScrollingDevicePrivate as their d_ptr. However, this was not enforced, and would fail for the "Virtual core pointer". To fix this, always use qobject_cast to verify that the device is of the correct type. Change-Id: I4a6b1d4d79308eb04e9f52dda00294fffe377bdf Reviewed-by: Volker Hilsheimer <[email protected]>
HoBoIs · Dec 23, 2020 · c80d647 · c80d647
1 parent 3729d37
commit c80d647
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 24 deletions.
diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h
@@ -78,7 +78,7 @@ class QXcbScreen;
 class QXcbWindow;
 class QXcbDrag;
 class QXcbKeyboard;
-class QXcbScrollingDevicePrivate;
+class QXcbScrollingDevice;
 class QXcbClipboard;
 class QXcbWMSupport;
 class QXcbNativeInterface;
@@ -316,8 +316,8 @@ class Q_XCB_EXPORT QXcbConnection : public QXcbBasicConnection
     TabletData *tabletDataForDevice(int id);
 #endif // QT_CONFIG(tabletevent)
     void xi2HandleScrollEvent(void *event, const QPointingDevice *scrollingDevice);
-    void xi2UpdateScrollingDevice(QXcbScrollingDevicePrivate *scrollingDevice);
-    QXcbScrollingDevicePrivate *scrollingDeviceForId(int id);
+    void xi2UpdateScrollingDevice(QInputDevice *scrollingDevice);
+    QXcbScrollingDevice *scrollingDeviceForId(int id);
 
     static bool xi2GetValuatorValueIfSet(const void *event, int valuatorNum, double *value);
 

diff --git a/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp b/src/plugins/platforms/xcb/qxcbconnection_xi2.cpp
@@ -433,7 +433,7 @@ void QXcbConnection::xi2SetupSlavePointerDevice(void *info, bool removeExisting,
             scrollingDeviceP->buttonCount = buttonCount;
             if (master)
                 scrollingDeviceP->seatName = master->seatName();
-            QWindowSystemInterface::registerInputDevice(new QXcbScrollingMouse(*scrollingDeviceP, master));
+            QWindowSystemInterface::registerInputDevice(new QXcbScrollingDevice(*scrollingDeviceP, master));
         } else {
             QWindowSystemInterface::registerInputDevice(new QPointingDevice(
                     name, deviceInfo->deviceid,
@@ -991,8 +991,13 @@ void QXcbConnection::xi2HandleDeviceChangedEvent(void *event)
     }
 }
 
-void QXcbConnection::xi2UpdateScrollingDevice(QXcbScrollingDevicePrivate *scrollingDevice)
+void QXcbConnection::xi2UpdateScrollingDevice(QInputDevice *dev)
 {
+    QXcbScrollingDevice *scrollDev = qobject_cast<QXcbScrollingDevice *>(dev);
+    if (!scrollDev || !scrollDev->capabilities().testFlag(QInputDevice::Capability::Scroll))
+        return;
+    QXcbScrollingDevicePrivate *scrollingDevice = QXcbScrollingDevice::get(scrollDev);
+
     auto reply = Q_XCB_REPLY(xcb_input_xi_query_device, xcb_connection(), scrollingDevice->systemId);
     if (!reply || reply->num_infos <= 0) {
         qCDebug(lcQpaXInputDevices, "scrolling device %lld no longer present", scrollingDevice->systemId);
@@ -1026,30 +1031,27 @@ void QXcbConnection::xi2UpdateScrollingDevices()
 {
     const auto &devices = QInputDevice::devices();
     for (const QInputDevice *dev : devices) {
-        if (dev->capabilities().testFlag(QInputDevice::Capability::Scroll)) {
-            const auto devPriv = QPointingDevicePrivate::get(static_cast<QPointingDevice *>(const_cast<QInputDevice *>(dev)));
-            xi2UpdateScrollingDevice(static_cast<QXcbScrollingDevicePrivate *>(devPriv));
-        }
+        if (dev->capabilities().testFlag(QInputDevice::Capability::Scroll))
+            xi2UpdateScrollingDevice(const_cast<QInputDevice *>(dev));
     }
 }
 
-QXcbScrollingDevicePrivate *QXcbConnection::scrollingDeviceForId(int id)
+QXcbScrollingDevice *QXcbConnection::scrollingDeviceForId(int id)
 {
     const QPointingDevice *dev = QPointingDevicePrivate::pointingDeviceById(id);
-    if (!dev)
+    if (!dev|| !dev->capabilities().testFlag(QInputDevice::Capability::Scroll))
         return nullptr;
-    if (!dev->capabilities().testFlag(QInputDevice::Capability::Scroll))
-        return nullptr;
-    auto devPriv = QPointingDevicePrivate::get(const_cast<QPointingDevice *>(dev));
-    return static_cast<QXcbScrollingDevicePrivate *>(devPriv);
+    return qobject_cast<QXcbScrollingDevice *>(const_cast<QPointingDevice *>(dev));
 }
 
 void QXcbConnection::xi2HandleScrollEvent(void *event, const QPointingDevice *dev)
 {
     auto *xiDeviceEvent = reinterpret_cast<qt_xcb_input_device_event_t *>(event);
-    if (!dev->capabilities().testFlag(QInputDevice::Capability::Scroll))
+
+    const QXcbScrollingDevice *scrollDev = qobject_cast<const QXcbScrollingDevice *>(dev);
+    if (!scrollDev || !scrollDev->capabilities().testFlag(QInputDevice::Capability::Scroll))
         return;
-    const auto scrollingDevice = static_cast<const QXcbScrollingDevicePrivate *>(QPointingDevicePrivate::get(dev));
+    const QXcbScrollingDevicePrivate *scrollingDevice = QXcbScrollingDevice::get(scrollDev);
 
     if (xiDeviceEvent->event_type == XCB_INPUT_MOTION && scrollingDevice->orientations) {
         if (QXcbWindow *platformWindow = platformWindowFromId(xiDeviceEvent->event)) {

diff --git a/src/plugins/platforms/xcb/qxcbscrollingdevice_p.h b/src/plugins/platforms/xcb/qxcbscrollingdevice_p.h
@@ -44,11 +44,6 @@
 
 QT_BEGIN_NAMESPACE
 
-/*! \internal
-    On the xcb platform, if a device's QPointingDevice::capabilities() includes
-    QInputDevice::Capability::Scroll, then its d-pointer must point to
-    an instance of this subclass, which tracks the scrolling valuators.
-*/
 class QXcbScrollingDevicePrivate : public QPointingDevicePrivate
 {
     Q_DECLARE_PUBLIC(QPointingDevice)
@@ -67,11 +62,23 @@ class QXcbScrollingDevicePrivate : public QPointingDevicePrivate
     // end of scrolling-related data
 };
 
-class QXcbScrollingMouse : public QPointingDevice
+class QXcbScrollingDevice : public QPointingDevice
 {
+    Q_OBJECT
 public:
-    QXcbScrollingMouse(QXcbScrollingDevicePrivate &d, QObject *parent)
+    QXcbScrollingDevice(QXcbScrollingDevicePrivate &d, QObject *parent)
         : QPointingDevice(d, parent) {}
+
+    inline static QXcbScrollingDevicePrivate *get(QXcbScrollingDevice *q)
+    {
+        return static_cast<QXcbScrollingDevicePrivate *>(QObjectPrivate::get(q));
+    }
+
+    inline static const QXcbScrollingDevicePrivate *get(const QXcbScrollingDevice *q)
+    {
+        return static_cast<const QXcbScrollingDevicePrivate *>(QObjectPrivate::get(q));
+    }
+
 };
 
 QT_END_NAMESPACE