From 47b5b2ea65ae03939280d5afe87f51faebca48a2 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Thu, 9 Apr 2026 06:38:58 +0530 Subject: [PATCH] Prevent reading uri-list items for a drag and drop in same window --- docs/dnd-protocol.rst | 9 +++++++++ kitty/dnd.c | 12 ++++++++++-- kitty_tests/dnd.py | 40 +++++++++++++++------------------------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/docs/dnd-protocol.rst b/docs/dnd-protocol.rst index d4aec9d7e..7097021f6 100644 --- a/docs/dnd-protocol.rst +++ b/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 ----------------- diff --git a/kitty/dnd.c b/kitty/dnd.c index 3fddfa4f5..2d4caa472 100644 --- a/kitty/dnd.c +++ b/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 diff --git a/kitty_tests/dnd.py b/kitty_tests/dnd.py index aee47d751..143227f32 100644 --- a/kitty_tests/dnd.py +++ b/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)