On Fri, Apr 03, 2026 at 08:12:37AM -0700, Z3rco wrote:

Summary

Thanks for the review.

Found and fixed several security issues across the C, Python, and Go code during a source audit. All changes are minimal and surgical — no refactoring, no behavior changes beyond the fixes themselves.

Timing-safe comparisons

• crypto.c — Secret.__eq__ used memcmp (not constant-time) and compared only min(len_a, len_b) bytes, so secrets of different lengths with a shared prefix compared equal. Switched to CRYPTO_memcmp with a length-equality check first.
• remote_control.py — Remote control passwords were looked up via dict.get(), which leaks timing through Python's hash+compare. Replaced with a constant-time linear scan using hmac.compare_digest.

This breaks type checking.

• file_transmission.py — Bypass token comparison in check_bypass used == for both the kitty-1 and sha256 protocols. Switched to hmac.compare_digest.

Memory safety (C)

• child-monitor.c — write_to_peer had an inverted condition (n > used instead of n < used). Since send() never returns more than used, the memmove never fired, so partial writes resent stale data from the buffer head.

This is a bug, sure. Don't see how it's a security issue.

• ibus_glfw.c — IBUS_ADDRESS env var copied via memcpy without a null terminator. When strlen(addr) >= PATH_MAX, the static buffer was returned unterminated.

Again dont see how this is a security issue. If the attacker can control the env vars the kitty process sees, its game over in a dozen different ways already.

• x11_window.c — Two realloc calls (clipboard + DnD chunked writer) didn't check for NULL, leading to a use-after-free of the old pointer and a memcpy into NULL + offset.

Again a bug, but a security issue? OOM will almost certainly end up terminating the process long before this could be exploited.

• dnd.c — accepted_mimes buffer had no size cap (unlike registered_mimes which caps at 1MB) and the realloc pattern leaked the old pointer on failure. Added the same 1MB cap and safe realloc.

Failure is OOM, leak of old memory doesnt really matter there as the process will die. As for no cap, yes but that's a potential DoS at best.

• png-reader.c — rowbytes * height was computed as png_uint_32 arithmetic, which can overflow on 32-bit before assigning to size_t. Added explicit casts.

The expression was sizeof(png_byte) * rowbytes * d->height and sizeof() evaluates to size_t.

Secrets hygiene

• disk-cache.c — CacheValue contains a 64-byte encryption key that was left in memory after free(). Added explicit_bzero before freeing.

explicit_bzero() is not portable. You need to use memset_s on macOS, for instance.

Tar extraction hardening

• tar.go — Hardlink targets were not validated against the destination prefix the way dest and symlinks were, allowing a malicious tar to hardlink outside the extraction directory then overwrite the target via a later entry. Also stripped setuid/setgid/sticky bits from extracted file modes.

Those bits are preserved by design, as should be obvious by the fact that they are explicitly enumerated. Also, ExtractAllFromTar is only used with tar files kitty itself creates, so not potentially malicious ones. That said, no harm in restricting hard links to be within the dest path.

Fair points. I used "security" loosely. These are correctness/robustness bugs, not exploitable vulnerabilities. The intent was to fix spots where the code didn't behave as written, not to claim active threat scenarios.

Yeah no worries, as I said thanks for the review. I am always happy to harden kitty code.