From 60da899bd9e490a0297ed67ee704b8243dc3c360 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Tue, 28 Apr 2026 17:19:20 +0300 Subject: [PATCH] fix: address open PR #105 review comments (leaks + null safety) - ui/window.c win_show_subwin (#1144): unconditional newpad without freeing the previous pad leaks the ncurses subwin if the function is invoked twice without an intervening win_hide_subwin. Added a delwin guard. - omemo/omemo.c omemo_on_disconnect (#1126): hash tables and libsignal handles are destroyed unconditionally even when a full init never ran (e.g. shutdown after a failed load), and neither g_hash_table_destroy nor signal_*_destroy are documented as NULL-safe. NULL-check each handle before tearing it down. - xmpp/roster_list.c (#1140 / #1141 / #1160): null the caller's pointer immediately after resource_destroy() so a stale reference cannot trigger a double-free; resource_destroy itself only zeroes the struct fields it owns and cannot reach back to the caller. --- src/omemo/omemo.c | 23 ++++++++++++++++++----- src/ui/window.c | 5 +++++ src/xmpp/roster_list.c | 2 ++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index 657266c9..b018158b 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -281,11 +281,24 @@ omemo_on_disconnect(void) } free_keyfile(&omemo_ctx.identity); - g_hash_table_destroy(omemo_ctx.known_devices); - g_hash_table_destroy(omemo_ctx.device_list_handler); - g_hash_table_destroy(omemo_ctx.device_list); - signal_protocol_store_context_destroy(omemo_ctx.store); - signal_context_destroy(omemo_ctx.signal); + /* These can be NULL when omemo_on_disconnect runs before a full + * init has populated them; g_hash_table_destroy and the libsignal + * destructors are not documented to be NULL-safe. */ + if (omemo_ctx.known_devices) { + g_hash_table_destroy(omemo_ctx.known_devices); + } + if (omemo_ctx.device_list_handler) { + g_hash_table_destroy(omemo_ctx.device_list_handler); + } + if (omemo_ctx.device_list) { + g_hash_table_destroy(omemo_ctx.device_list); + } + if (omemo_ctx.store) { + signal_protocol_store_context_destroy(omemo_ctx.store); + } + if (omemo_ctx.signal) { + signal_context_destroy(omemo_ctx.signal); + } memset(&omemo_ctx, 0, sizeof(omemo_ctx)); wins_omemo_trust_changed(NULL); diff --git a/src/ui/window.c b/src/ui/window.c index 6fb46ac1..7624c22c 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -577,6 +577,11 @@ win_show_subwin(ProfWin* window) } ProfLayoutSplit* layout = (ProfLayoutSplit*)window->layout; + /* Free a previous pad if win_show_subwin was called twice in a row + * without an intervening win_hide_subwin. */ + if (layout->subwin) { + delwin(layout->subwin); + } layout->subwin = newpad(PAD_MIN_HEIGHT, subwin_cols); wbkgd(layout->subwin, theme_attrs(THEME_TEXT)); wresize(layout->base.win, PAD_MIN_HEIGHT, cols - subwin_cols); diff --git a/src/xmpp/roster_list.c b/src/xmpp/roster_list.c index 47be2f19..0a9ea14d 100644 --- a/src/xmpp/roster_list.c +++ b/src/xmpp/roster_list.c @@ -88,6 +88,7 @@ _pendingPresence_free(ProfPendingPresence* presence) g_date_time_unref(presence->last_activity); free(presence->barejid); resource_destroy(presence->resource); + presence->resource = NULL; free(presence); } @@ -136,6 +137,7 @@ roster_update_presence(const char* const barejid, Resource* resource, GDateTime* if (contact == NULL) { /* Don't lose resource when there is no owner. */ resource_destroy(resource); + resource = NULL; return FALSE; } if (!_datetimes_equal(p_contact_last_activity(contact), last_activity)) {