From 711c5bb43f2823766d5189dc8d567c8f4cec253c Mon Sep 17 00:00:00 2001
From: David Edmundson <kde@davidedmundson.co.uk>
Date: Wed, 15 May 2024 22:08:26 +0100
Subject: [PATCH] wayland: send dndFinished to source if target fails to do so

After receiving a drop a client should call data_offer.finish
to tell the source it's done using the drop.

A client could delete an offer after drop before calling finish.

This can happen with misbehaving/buggy or malicious Wayland clients.
A real case was found in the wild with Chromium, as described in the
linked bug.

In this situation we should let the source know the dnd is finished
as there are no other transfers than can take place.

We don't want to universally send this in all data offer destructors
only, offers that are deleted post drop so the flag on the source is
exposed.

BUG: 482142
---
 src/wayland/abstract_data_source.h | 24 ++++++++++++++++++++++--
 src/wayland/dataoffer.cpp          |  9 ++++++++-
 src/wayland/datasource.cpp         | 15 +++------------
 src/wayland/datasource.h           |  3 ---
 4 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/src/wayland/abstract_data_source.h b/src/wayland/abstract_data_source.h
index d5213d8f8a0..af5f9005bdb 100644
--- a/src/wayland/abstract_data_source.h
+++ b/src/wayland/abstract_data_source.h
@@ -56,11 +56,16 @@ public:
     /**
      * The user performed the drop action during a drag and drop operation.
      */
-    virtual void dropPerformed(){};
+    virtual void dropPerformed()
+    {
+        m_dndDropped = true;
+    }
     /**
      * The drop destination finished interoperating with this data source.
      */
-    virtual void dndFinished(){};
+    virtual void dndFinished()
+    {
+    }
     /**
      * This event indicates the @p action selected by the compositor after matching the
      * source/destination side actions. Only one action (or none) will be offered here.
@@ -69,11 +74,22 @@ public:
     {
     };

+    bool isDndCancelled() const
+    {
+        return m_dndCancelled;
+    }
+
+    bool isDropPerformed() const
+    {
+        return m_dndDropped;
+    }
+
     /**
      *  Called when a user stops clicking but it is not accepted by a client.
      */
     virtual void dndCancelled()
     {
+        m_dndCancelled = true;
     }

     virtual wl_client *client() const
@@ -89,6 +105,10 @@ Q_SIGNALS:

 protected:
     explicit AbstractDataSource(QObject *parent = nullptr);
+
+private:
+    bool m_dndCancelled = false;
+    bool m_dndDropped = false;
 };

 }
diff --git a/src/wayland/dataoffer.cpp b/src/wayland/dataoffer.cpp
index 91f5f552b93..79d8f32034f 100644
--- a/src/wayland/dataoffer.cpp
+++ b/src/wayland/dataoffer.cpp
@@ -77,6 +77,7 @@ void DataOfferInterfacePrivate::data_offer_finish(Resource *resource)
         return;
     }
     source->dndFinished();
+    source.clear();
     // TODO: It is a client error to perform other requests than wl_data_offer.destroy after this one
 }

@@ -163,7 +164,13 @@ DataOfferInterface::DataOfferInterface(AbstractDataSource *source, wl_resource *
     });
 }

-DataOfferInterface::~DataOfferInterface() = default;
+DataOfferInterface::~DataOfferInterface()
+{
+    if (d->source && d->source->isDropPerformed()) {
+        d->source->dndFinished();
+        d->source.clear();
+    }
+}

 void DataOfferInterface::sendAllOffers()
 {
diff --git a/src/wayland/datasource.cpp b/src/wayland/datasource.cpp
index 418a160e01e..1713c135c37 100644
--- a/src/wayland/datasource.cpp
+++ b/src/wayland/datasource.cpp
@@ -129,7 +129,7 @@ DataDeviceManagerInterface::DnDAction DataSourceInterface::selectedDndAction() c

 void DataSourceInterface::dropPerformed()
 {
-    d->dropPerformed = true;
+    AbstractDataSource::dropPerformed();
     if (d->resource()->version() < WL_DATA_SOURCE_DND_DROP_PERFORMED_SINCE_VERSION) {
         return;
     }
@@ -138,17 +138,13 @@ void DataSourceInterface::dropPerformed()

 void DataSourceInterface::dndFinished()
 {
+    AbstractDataSource::dndFinished();
     if (d->resource()->version() < WL_DATA_SOURCE_DND_FINISHED_SINCE_VERSION) {
         return;
     }
     d->send_dnd_finished();
 }

-bool DataSourceInterface::isDropPerformed() const
-{
-    return d->dropPerformed;
-}
-
 void DataSourceInterface::dndAction(DataDeviceManagerInterface::DnDAction action)
 {
     d->selectedDndAction = action;
@@ -169,7 +165,7 @@ void DataSourceInterface::dndAction(DataDeviceManagerInterface::DnDAction action

 void DataSourceInterface::dndCancelled()
 {
-    d->isCanceled = true;
+    AbstractDataSource::dndCancelled();
     // for v3 or less, cancel should not be called after a failed drag operation
     if (wl_resource_get_version(resource()) < 3) {
         return;
@@ -177,11 +173,6 @@ void DataSourceInterface::dndCancelled()
     d->send_cancelled();
 }

-bool DataSourceInterface::isDndCancelled() const
-{
-    return d->isCanceled;
-}
-
 wl_resource *DataSourceInterface::resource() const
 {
     return d->resource()->handle;
diff --git a/src/wayland/datasource.h b/src/wayland/datasource.h
index dc39556ed62..e19d3cf4cd0 100644
--- a/src/wayland/datasource.h
+++ b/src/wayland/datasource.h
@@ -43,9 +43,6 @@ public:
     void dndAction(DataDeviceManagerInterface::DnDAction action) override;
     void dndCancelled() override;

-    bool isDndCancelled() const;
-    bool isDropPerformed() const;
-
     wl_resource *resource() const;

     wl_client *client() const override;
--
GitLab