From b39f88c6a2e810f3e55e919b95bf159ec05ef2df Mon Sep 17 00:00:00 2001 From: z3rco Date: Fri, 3 Apr 2026 16:10:46 +0100 Subject: [PATCH] Fix multiple security vulnerabilities across C, Python, and Go code Timing-safe comparisons: - crypto.c: Replace memcmp with CRYPTO_memcmp for Secret equality, require equal lengths before comparing - remote_control.py: Constant-time password lookup to avoid leaking valid passwords via dict hash timing - file_transmission.py: Use hmac.compare_digest for bypass token comparison instead of == Memory safety: - child-monitor.c: Fix inverted condition in write_to_peer that prevented memmove from ever executing on partial writes - ibus_glfw.c: Null-terminate IBUS_ADDRESS copy to prevent string overread when strlen >= PATH_MAX - x11_window.c: Add NULL checks after realloc in clipboard/DnD data handling (two sites) - dnd.c: Cap accepted_mimes at 1MB to prevent unbounded growth, fix realloc to not lose the original pointer on failure - png-reader.c: Cast to size_t before multiplication to prevent integer overflow on 32-bit platforms Secrets hygiene: - disk-cache.c: Zero encryption_key with explicit_bzero before free Tar extraction hardening: - tar.go: Validate hardlink targets against destination prefix to prevent writing outside extraction directory - tar.go: Strip setuid/setgid/sticky bits from extracted files Co-Authored-By: Claude Opus 4.6 (1M context) --- glfw/ibus_glfw.c | 4 +++- glfw/x11_window.c | 8 ++++++-- kitty/child-monitor.c | 2 +- kitty/crypto.c | 5 +++-- kitty/disk-cache.c | 3 ++- kitty/dnd.c | 11 ++++++----- kitty/file_transmission.py | 5 +++-- kitty/png-reader.c | 2 +- kitty/remote_control.py | 15 ++++++++++++--- tools/utils/tar.go | 8 +++++++- 10 files changed, 44 insertions(+), 19 deletions(-) diff --git a/glfw/ibus_glfw.c b/glfw/ibus_glfw.c index 081987ff5..10bc3d40b 100644 --- a/glfw/ibus_glfw.c +++ b/glfw/ibus_glfw.c @@ -287,7 +287,9 @@ get_ibus_address_file_name(void) { addr = getenv("IBUS_ADDRESS"); int offset = 0; if (addr && addr[0]) { - memcpy(ans, addr, GLFW_MIN(strlen(addr), sizeof(ans))); + size_t len = GLFW_MIN(strlen(addr), sizeof(ans) - 1); + memcpy(ans, addr, len); + ans[len] = '\0'; return ans; } const char* disp_num = NULL; diff --git a/glfw/x11_window.c b/glfw/x11_window.c index b8d1f6f83..fe2ea0af5 100644 --- a/glfw/x11_window.c +++ b/glfw/x11_window.c @@ -937,7 +937,9 @@ get_clipboard_data(const _GLFWClipboardData *cd, const char *mime, char **data) if (!chunk.sz) break; if (cap < sz + chunk.sz) { cap = MAX(cap * 2, sz + 4 * chunk.sz); - buf = realloc(buf, cap * sizeof(buf[0])); + char *new_buf = realloc(buf, cap * sizeof(buf[0])); + if (!new_buf) { free(buf); *data = NULL; cd->get_data(NULL, iter, cd->ctype); return 0; } + buf = new_buf; } memcpy(buf + sz, chunk.data, chunk.sz); sz += chunk.sz; @@ -3636,7 +3638,9 @@ write_chunk(void *object, const char *data, size_t sz) { if (data) { if (cw->cap < cw->sz + sz) { cw->cap = MAX(cw->cap * 2, cw->sz + 8*sz); - cw->buf = realloc(cw->buf, cw->cap * sizeof(cw->buf[0])); + char *new_buf = realloc(cw->buf, cw->cap * sizeof(cw->buf[0])); + if (!new_buf) return false; + cw->buf = new_buf; } memcpy(cw->buf + cw->sz, data, sz); cw->sz += sz; diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index c26995561..9ce3d0b35 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -1940,7 +1940,7 @@ write_to_peer(Peer *peer) { else if (n < 0) { if (errno != EINTR) { log_error("write() to peer socket failed with error: %s", strerror(errno)); peer->write.used = 0; peer->write.failed = true; } } else { - if ((size_t)n > peer->write.used) memmove(peer->write.data, peer->write.data + n, peer->write.used - n); + if ((size_t)n < peer->write.used) memmove(peer->write.data, peer->write.data + n, peer->write.used - n); peer->write.used -= n; } talk_mutex(unlock); diff --git a/kitty/crypto.c b/kitty/crypto.c index 002b795c9..e8a957789 100644 --- a/kitty/crypto.c +++ b/kitty/crypto.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -72,8 +73,8 @@ dealloc_secret(Secret *self) { static int __eq__(Secret *a, Secret *b) { - const size_t l = a->secret_len < b->secret_len ? a->secret_len : b->secret_len; - return memcmp(a->secret, b->secret, l) == 0; + if (a->secret_len != b->secret_len) return 0; + return CRYPTO_memcmp(a->secret, b->secret, a->secret_len) == 0; } static Py_ssize_t diff --git a/kitty/disk-cache.c b/kitty/disk-cache.c index 83e85bb65..995378a1b 100644 --- a/kitty/disk-cache.c +++ b/kitty/disk-cache.c @@ -16,6 +16,7 @@ #include "cross-platform-random.h" #include #include +#include #include #include #include @@ -40,7 +41,7 @@ static uint64_t key_hash(KEY_TY k); #define HASH_FN key_hash static bool keys_are_equal(CacheKey a, CacheKey b) { return a.hash_keylen == b.hash_keylen && memcmp(a.hash_key, b.hash_key, a.hash_keylen) == 0; } #define CMPR_FN keys_are_equal -static void free_cache_value(CacheValue *cv) { free(cv->data); cv->data = NULL; free(cv); } +static void free_cache_value(CacheValue *cv) { explicit_bzero(cv->encryption_key, sizeof(cv->encryption_key)); free(cv->data); cv->data = NULL; free(cv); } static void free_cache_key(CacheKey cv) { free(cv.hash_key); cv.hash_key = NULL; } #define KEY_DTOR_FN free_cache_key #define VAL_DTOR_FN free_cache_value diff --git a/kitty/dnd.c b/kitty/dnd.c index 7f68dd8f1..0d5bb6c7d 100644 --- a/kitty/dnd.c +++ b/kitty/dnd.c @@ -308,11 +308,12 @@ drop_set_status(Window *w, int operation, const char *payload, size_t payload_sz } } if (payload_sz) { - w->drop.accepted_mimes = realloc(w->drop.accepted_mimes, w->drop.accepted_mimes_sz + payload_sz + 2); - if (w->drop.accepted_mimes) { - memcpy(w->drop.accepted_mimes + w->drop.accepted_mimes_sz, payload, payload_sz); - w->drop.accepted_mimes_sz += payload_sz; - } + if (w->drop.accepted_mimes_sz + payload_sz > 1024 * 1024) return; + char *new_buf = realloc(w->drop.accepted_mimes, w->drop.accepted_mimes_sz + payload_sz + 2); + if (!new_buf) return; + w->drop.accepted_mimes = new_buf; + memcpy(w->drop.accepted_mimes + w->drop.accepted_mimes_sz, payload, payload_sz); + w->drop.accepted_mimes_sz += payload_sz; } if (!more) { w->drop.accept_in_progress = false; diff --git a/kitty/file_transmission.py b/kitty/file_transmission.py index 2131063d8..374d9b7af 100644 --- a/kitty/file_transmission.py +++ b/kitty/file_transmission.py @@ -2,6 +2,7 @@ # License: GPLv3 Copyright: 2021, Kovid Goyal import errno +import hmac import inspect import io import json @@ -572,12 +573,12 @@ def check_bypass(password: str, request_id: str, bypass_data: str) -> bool: delta = time_ns() - int(timestamp) if abs(delta) > 5 * 60 * 1e9: return False - return payload == f'{request_id};{password}' + return hmac.compare_digest(payload, f'{request_id};{password}') except Exception as err: log_error(f'Invalid file transmission bypass data received: {err}') return False elif protocol == 'sha256': - return (encode_bypass(request_id, password) == bypass_data) if password else False + return hmac.compare_digest(encode_bypass(request_id, password), bypass_data) if password else False else: log_error(f'Invalid file transmission bypass data received with protocol: {protocol}') return False diff --git a/kitty/png-reader.c b/kitty/png-reader.c index 073536c72..b6bd56ff6 100644 --- a/kitty/png-reader.c +++ b/kitty/png-reader.c @@ -113,7 +113,7 @@ inflate_png_inner(png_read_data *d, const uint8_t *buf, size_t bufsz, int max_im png_read_update_info(png, info); png_uint_32 rowbytes = png_get_rowbytes(png, info); - d->sz = sizeof(png_byte) * rowbytes * d->height; + d->sz = (size_t)rowbytes * (size_t)d->height; d->decompressed = malloc(d->sz + 16); if (d->decompressed == NULL) ABRT(ENOMEM, "Out of memory allocating decompression buffer for PNG"); d->row_pointers = malloc(d->height * sizeof(png_bytep)); diff --git a/kitty/remote_control.py b/kitty/remote_control.py index fb6a9f94f..183d9550d 100644 --- a/kitty/remote_control.py +++ b/kitty/remote_control.py @@ -2,6 +2,7 @@ # License: GPL v3 Copyright: 2018, Kovid Goyal import base64 +import hmac import json import os import re @@ -103,6 +104,14 @@ def fnmatch_pattern(pat: str) -> 're.Pattern[str]': return re.compile(translate(pat)) +def _ct_password_lookup(passwords: dict[str, Any], pw: str) -> Any: + result = None + for k, v in passwords.items(): + if hmac.compare_digest(k, pw): + result = v + return result + + def remote_control_allowed( pcmd: dict[str, Any], remote_control_passwords: dict[str, Sequence[str]] | None, window: Optional['Window'], extra_data: dict[str, Any] @@ -110,7 +119,7 @@ def remote_control_allowed( if not remote_control_passwords: return True pw = pcmd.get('password', '') - auth_items = remote_control_passwords.get(pw) + auth_items = _ct_password_lookup(remote_control_passwords, pw) if pw == '!': auth_items = None if auth_items is None: @@ -185,10 +194,10 @@ def is_cmd_allowed(pcmd: dict[str, Any], window: Optional['Window'], from_socket return False pa = password_authorizer(auth_items) return pa.is_cmd_allowed(pcmd, window, from_socket, extra_data) - q = user_password_allowed.get(pw) + q = _ct_password_lookup(user_password_allowed, pw) if q is not None: return q - auth_items = get_options().remote_control_password.get(pw) + auth_items = _ct_password_lookup(get_options().remote_control_password, pw) if auth_items is None: return None pa = password_authorizer(auth_items) diff --git a/tools/utils/tar.go b/tools/utils/tar.go index af1ba0473..9cdf4f72d 100644 --- a/tools/utils/tar.go +++ b/tools/utils/tar.go @@ -182,7 +182,7 @@ func ExtractAllFromTar(tr *tar.Reader, dest_path string, optss ...TarExtractOpti dest_path = filepath.Clean(dest_path) mode := func(hdr int64) fs.FileMode { - return fs.FileMode(hdr) & (fs.ModePerm | fs.ModeSetgid | fs.ModeSetuid | fs.ModeSticky) + return fs.FileMode(hdr) & fs.ModePerm } set_metadata := func(chmod func(mode fs.FileMode) error, hdr_mode int64) (err error) { @@ -250,6 +250,12 @@ func ExtractAllFromTar(tr *tar.Reader, dest_path string, optss ...TarExtractOpti if !filepath.IsAbs(link_target) { link_target = filepath.Join(filepath.Dir(dest), link_target) } + if link_target, err = EvalSymlinksThatExist(link_target); err != nil { + return + } + if !strings.HasPrefix(link_target, needed_prefix) { + continue + } if err = os.Link(link_target, dest); err != nil { return }