Prevent reading uri-list items for a drag and drop in same window

docs/dnd-protocol.rst

@@ -158,6 +158,12 @@ must respond with ``t=R ; EINVAL`` if the file is not a regular file after
 resolving symlinks and ``t=R ; ENOENT`` if the file does not exist. If an
 I/O error occurs the terminal must send ``t=R ; EIO``.
 
+For security reasons, terminals must reply with ``t=R ; EPERM`` if the drag
+originated in the same window as the drop, this prevents malicious programs
+from reading files on the computer by starting their own drag. This is a
+defense in depth feature since drags can only be started by the terminal, but
+it helps in case of accidental drag starts and drops into the same window.
+
 
 Reading remote directories
 +++++++++++++++++++++++++++
@@ -309,6 +315,9 @@ If the client wants to cancel the full drag at any time, it should send:
 
     OSC _dnd_code ; t=E:y=-1 ST
 
+If ``t=e`` or ``t=E`` escape codes are sent to the terminal before the drag is
+started and the terminal replies with ``t=R ; OK``, the terminal must respond
+with ``t=R ; EINVAL`` and abort the drag.
 
 Multiplexers
 -----------------

kitty/dnd.c

@@ -752,6 +752,9 @@ drop_request_uri_data(Window *w, const char *payload, size_t payload_sz) {
     if (!w->drop.uri_list || !w->drop.uri_list_sz) {
         drop_send_error(w, EINVAL); return;
     }
+    if (global_state.drag_source.from_window == w->id && w->drag_source.state != DRAG_SOURCE_NONE) {
+        drop_send_error(w, EPERM); return;
+    }
 
     /* Payload format: "text/uri-list:idx" */
     const char *colon = memchr(payload, ':', payload_sz);
@@ -1070,6 +1073,7 @@ drag_start(Window *w) {
 
 void
 drag_notify(Window *w, DragNotifyType type) {
+    if (ds.state < DRAG_SOURCE_STARTED) return;
     char buf[128];
     size_t sz = snprintf(buf, sizeof(buf), "t=e:x=%d", type + 1);
     switch(type) {
@@ -1091,6 +1095,7 @@ drag_notify(Window *w, DragNotifyType type) {
             sz += snprintf(buf + sz, sizeof(buf) - sz, "y=%d", global_state.drag_source.was_canceled ? 1 : 0); break;
     }
     queue_payload_to_child(w->id, w->drag_source.client_id, &w->drag_source.pending, buf, sz, NULL, 0, false);
+    if (type == DRAG_NOTIFY_FINISHED) drag_free_offer(w);
 }
 
 int
@@ -1103,7 +1108,7 @@ drag_free_data(Window *w, const char *mime_type, const char* data, size_t sz) {
 const char*
 drag_get_data(Window *w, const char *mime_type, size_t *sz, int *err_code) {
     *err_code = ENOENT; *sz = 0;
-    if (!ds.items) return NULL;
+    if (!ds.items || ds.state < DRAG_SOURCE_DROPPED) return NULL;
     for (size_t i = 0; i < ds.num_mimes; i++) {
         if (strcmp(ds.items[i].mime_type, mime_type) == 0) {
             if (ds.items[i].fd_plus_one < 0) {
@@ -1186,7 +1191,10 @@ open_item_tmpfile(void) {
 
 void
 drag_process_item_data(Window *w, size_t idx, int has_more, const uint8_t *payload, size_t payload_sz) {
-    if ((ds.state != DRAG_SOURCE_STARTED && ds.state != DRAG_SOURCE_DROPPED) || idx >= ds.num_mimes || !ds.items) return;
+    if ((ds.state < DRAG_SOURCE_DROPPED) || idx >= ds.num_mimes || !ds.items) {
+        abrt(EINVAL);
+        return;
+    }
 
     if (has_more < 0) {
         // Error from the client program

kitty_tests/dnd.py

@@ -1297,22 +1297,22 @@ class TestDnDProtocol(BaseTest):
             parse_bytes(screen, final_img)
             self._assert_no_output(cap, wid)
 
-    def test_drag_process_item_data_without_started_state_ignored(self) -> None:
+    def test_drag_process_item_data_without_started_state_invalid(self) -> None:
         """Sending t=e data before the drag is started is silently ignored."""
         with dnd_test_window() as (osw, wid, screen, cap):
             self._setup_drag_offer(screen, wid, cap, 'text/plain')
             # State is BEING_BUILT, not STARTED – drag_process_item_data should return early
             data_b64 = standard_b64encode(b'premature data').decode()
             parse_bytes(screen, client_drag_send_data(0, data_b64))
-            self._assert_no_output(cap, wid)
+            self.assert_error(cap, wid)
 
-    def test_drag_error_from_client_without_started_state_ignored(self) -> None:
+    def test_drag_error_from_client_without_started_state_invalid(self) -> None:
         """Sending t=E with a MIME index before the drag is started is silently ignored."""
         with dnd_test_window() as (osw, wid, screen, cap):
             self._setup_drag_offer(screen, wid, cap, 'text/plain')
             # State is BEING_BUILT – sending an error for index 0 should be ignored
             parse_bytes(screen, client_drag_send_error(0, 'EIO'))
-            self._assert_no_output(cap, wid)
+            self.assert_error(cap, wid)
 
     def test_drag_offer_with_empty_mimes_after_cancel(self) -> None:
         """After cancelling, a new offer can be started from scratch."""
@@ -1354,10 +1354,13 @@ class TestDnDProtocol(BaseTest):
 
             # Attempting to start should fail since unregister called drag_free_offer
             parse_bytes(screen, client_drag_start())
-            events = self._get_events(cap, wid)
-            self.assertEqual(len(events), 1, events)
-            self.ae(events[0]['type'], 'R')
-            self.ae(events[0]['payload'].strip(), b'EINVAL')
+            self.assert_error(cap, wid)
+
+    def assert_error(self, cap, wid, code='EINVAL'):
+        events = self._get_events(cap, wid)
+        self.assertEqual(len(events), 1, events)
+        self.ae(events[0]['type'], 'R')
+        self.ae(events[0]['payload'].strip(), code.encode())
 
     def test_drag_pre_send_multiple_mimes(self) -> None:
         """Pre-sent data can be provided for multiple different MIME types."""
@@ -1429,10 +1432,7 @@ class TestDnDProtocol(BaseTest):
             self._setup_drag_offer(screen, wid, cap, 'text/plain')
             # Send completely invalid base64
             parse_bytes(screen, client_drag_pre_send(0, '!@#$%^&*()'))
-            events = self._get_events(cap, wid)
-            self.assertEqual(len(events), 1, events)
-            self.ae(events[0]['type'], 'R')
-            self.ae(events[0]['payload'].strip(), b'EINVAL')
+            self.assert_error(cap, wid)
 
     def test_drag_add_image_invalid_base64_returns_einval(self) -> None:
         """Adding an image with invalid base64 data returns EINVAL."""
@@ -1440,10 +1440,7 @@ class TestDnDProtocol(BaseTest):
             self._setup_drag_offer(screen, wid, cap, 'text/plain')
             # Invalid base64 as image data
             parse_bytes(screen, client_drag_add_image(1, 32, 1, 1, '!@#$%^&*()'))
-            events = self._get_events(cap, wid)
-            self.assertEqual(len(events), 1, events)
-            self.ae(events[0]['type'], 'R')
-            self.ae(events[0]['payload'].strip(), b'EINVAL')
+            self.assert_error(cap, wid)
 
     def test_drag_start_with_image_size_mismatch(self) -> None:
         """Starting a drag when image data size doesn't match dimensions returns EINVAL."""
@@ -1459,10 +1456,7 @@ class TestDnDProtocol(BaseTest):
             # Actually no - for fmt=32, expand_rgb_data is not called, only for fmt=24.
             # The check img.sz != width*height*4 happens in drag_start.
             parse_bytes(screen, client_drag_start())
-            events = self._get_events(cap, wid)
-            self.assertEqual(len(events), 1, events)
-            self.ae(events[0]['type'], 'R')
-            self.ae(events[0]['payload'].strip(), b'EINVAL')
+            self.assert_error(cap, wid)
 
     def test_drag_start_with_rgb_image_size_mismatch(self) -> None:
         """Starting a drag when RGB image data size doesn't match w*h*3 returns EINVAL."""
@@ -1474,8 +1468,4 @@ class TestDnDProtocol(BaseTest):
             parse_bytes(screen, client_drag_add_image(1, 24, 2, 2, data_b64))
             # drag_start calls expand_rgb_data which checks sz == w*h*3
             parse_bytes(screen, client_drag_start())
-            events = self._get_events(cap, wid)
-            self.assertEqual(len(events), 1, events)
-            self.ae(events[0]['type'], 'R')
-            self.ae(events[0]['payload'].strip(), b'EINVAL')
-
+            self.assert_error(cap, wid)