From e89c0a7fb54a15b8df1d2938e2f01ef14f3bf7e5 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 22 Mar 2025 17:25:12 +0100 Subject: [PATCH] QWidgetWindow: fix UB (invalid downcast) in Private::handleDragEnterEvent() The handleDragEnterEvent() function is not only called for QEvent::DragEnter, but also, in handleDragMoveEvent(), for QEvent::DragMove, in which case the fully-derived event passed as an argument is a QDragMoveEvent, and not its subclass QDragEnterEvent. Code in Qt seems to assume that it's ok to cast an object down to a more-derived class than the most-derived dynamic type, as long as no extra state is being added between the two, but C++ does not chare that view. Says UBSan: qwidgetwindow.cpp:1000:38: runtime error: downcast of address 0x7ffe7b34c6e0 which does not point to an object of type 'QDragEnterEvent' 0x7ffe7b34c6e0: note: object is of type 'QDragMoveEvent' 00 00 00 00 e0 62 70 42 aa 7f 00 00 3d 00 00 00 00 00 00 00 00 00 00 00 00 c0 58 40 00 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QDragMoveEvent' Furtunately, handleDragEnterEvent() doesn't actually need its argument to be a QDragEnterEvent; QDragMoveEvent suffices, so we can just change the argument type back to QDragMoveEvent and remove the cast in handleDragMoveEvent(). Add a bit of \internal docs to describe the discrepancy between the function's name and argument type. Amends f8944a7f07112c85dc4f66848cabb490514cd28e. Picking all the way, because this is risk-less: Either the code compiles and then it works as before (minus the UB), or it doesn't compile. Pick-to: 6.9 6.8 6.5 5.15 Change-Id: I5be53b023d389902b43e9a896d074edea1c4ff2d Reviewed-by: Axel Spoerl --- src/widgets/kernel/qwidgetwindow.cpp | 14 ++++++++++++-- src/widgets/kernel/qwidgetwindow_p.h | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp index 98aed138aee..4cf2934a2dc 100644 --- a/src/widgets/kernel/qwidgetwindow.cpp +++ b/src/widgets/kernel/qwidgetwindow.cpp @@ -948,7 +948,17 @@ static QWidget *findDnDTarget(QWidget *parent, const QPoint &pos) return widget; } -void QWidgetWindow::handleDragEnterEvent(QDragEnterEvent *event, QWidget *widget) +/*! + \internal + + Sends \a event to \a widget. + + Also called from dragMoveEvent(), in which case \a event is-a + QDragMoveEvent only, not a full QDragEnterEvent, which is why this function + takes \a event as a QDragMoveEvent and not, as one would expect, + QDragEnterEvent (downcast would be UB). +*/ +void QWidgetWindow::handleDragEnterEvent(QDragMoveEvent *event, QWidget *widget) { Q_ASSERT(m_dragTarget == nullptr); if (!widget) @@ -997,7 +1007,7 @@ void QWidgetWindow::handleDragMoveEvent(QDragMoveEvent *event) // widget might have been deleted when handling the leaveEvent if (widget) { // Send DragEnter to new widget. - handleDragEnterEvent(static_cast(event), widget); + handleDragEnterEvent(event, widget); // Handling 'DragEnter' should suffice for the application. translated.setDropAction(event->dropAction()); translated.setAccepted(event->isAccepted()); diff --git a/src/widgets/kernel/qwidgetwindow_p.h b/src/widgets/kernel/qwidgetwindow_p.h index 717541142cd..342794ee3d3 100644 --- a/src/widgets/kernel/qwidgetwindow_p.h +++ b/src/widgets/kernel/qwidgetwindow_p.h @@ -65,7 +65,7 @@ protected: void handleWheelEvent(QWheelEvent *); #endif #if QT_CONFIG(draganddrop) - void handleDragEnterEvent(QDragEnterEvent *, QWidget *widget = nullptr); + void handleDragEnterEvent(QDragMoveEvent *, QWidget *widget = nullptr); void handleDragMoveEvent(QDragMoveEvent *); void handleDragLeaveEvent(QDragLeaveEvent *); void handleDropEvent(QDropEvent *);