Skip to content

Fix DnD memory safety issues, protocol output bugs, and undefined behavior#9869

Merged
kovidgoyal merged 2 commits intomasterfrom
copilot/fix-memory-leaks-and-add-tests
Apr 14, 2026
Merged

Fix DnD memory safety issues, protocol output bugs, and undefined behavior#9869
kovidgoyal merged 2 commits intomasterfrom
copilot/fix-memory-leaks-and-add-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

Thorough audit of the DnD protocol implementation (dnd.c) revealed memory leaks, a buffer overflow, protocol output bugs, and undefined behavior across several code paths.

Protocol output bugs

  • drag_notify missing OSC prefix — output was t=e:x=1:y=0\x1b\\ instead of \x1b]CODE;t=e:x=1:y=0\x1b\\, making notifications unparseable by clients
  • drag_notify missing colon separators — produced t=e:x=1y=0 instead of t=e:x=1:y=0

Memory leaks

  • drag_free_remote_itemchildren array pointer never freed after recursive child cleanup
  • drag_free_offerfree_pending(&ds.pending) never called, leaking queued unflushed entries
  • toplevel_data_for_drag — old strdup'd URI overwritten without free():
    // Before: leak
    mi.uri_list[uri_item_idx] = as_file_url(...);
    // After:
    free(mi.uri_list[uri_item_idx]);
    mi.uri_list[uri_item_idx] = as_file_url(...);
  • Realloc anti-pattern in 7 call sites (drop_register_window, drop_dispatch_data, drag_add_mimes, drag_add_pre_sent_data, drag_add_image, add_payload, queue_payload_to_child) — ptr = realloc(ptr, ...) loses the old pointer on failure

Buffer overflow

  • add_payload symlink caseri->data[ri->data_sz] = 0 writes past allocation when decoded data fills capacity exactly. Now ensures data_capacity >= data_sz + 1 before the null terminator write.

Integer overflow

  • expand_rgb_dataimg.width * img.height * 4 used int arithmetic before size_t conversion. Fixed with explicit (size_t) casts.

Undefined behavior

  • queue_payload_to_childmemcpy(dst, NULL, 0) when data is NULL and data_sz is 0. Guarded with size check.

Tests

  • 5 new tests exercising the drag_notify colon separator fix, children array cleanup, and URI replacement. Added dnd_test_drag_notify C test helper to invoke drag_notify with controlled parameters.

Copilot AI and others added 2 commits April 14, 2026 09:33
…entation

Fixes:
- Memory leak: drag_free_remote_item now frees children array
- Memory leak: drag_free_offer now frees pending data
- Memory leak: toplevel_data_for_drag frees old URI string before replacement
- Protocol bug: drag_notify missing OSC prefix in escape code output
- Protocol bug: drag_notify missing colon separators between metadata keys
- Buffer overflow: add_payload symlink case ensures capacity for null terminator
- Integer overflow: expand_rgb_data uses size_t casts for multiplication
- Memory leak: realloc anti-pattern fixed in 7 locations (drop_register_window,
  drop_dispatch_data, drag_add_mimes, drag_add_pre_sent_data, drag_add_image,
  add_payload default case, queue_payload_to_child)
- UB: queue_payload_to_child guards memcpy with NULL source + 0 count

Agent-Logs-Url: https://github.com/kovidgoyal/kitty/sessions/3c7e550c-e8e8-413e-a54b-87d61cb8e574

Co-authored-by: kovidgoyal <1308621+kovidgoyal@users.noreply.github.com>
Agent-Logs-Url: https://github.com/kovidgoyal/kitty/sessions/3c7e550c-e8e8-413e-a54b-87d61cb8e574

Co-authored-by: kovidgoyal <1308621+kovidgoyal@users.noreply.github.com>
@kovidgoyal kovidgoyal merged commit a9e56d5 into master Apr 14, 2026
22 checks passed
@kovidgoyal kovidgoyal deleted the copilot/fix-memory-leaks-and-add-tests branch April 14, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants