From a9c21ce487990fd2683603b4bd750bf4f7eeaadf Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Wed, 5 Nov 2025 14:39:10 +0300 Subject: [PATCH 1/7] fix(ui,db): harden NULL handling and resource lifecycle across UI and SQLite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ui/window: fix subwindow lifecycle (safe delwin on recreate), clamp widths, add fallback timestamp when loading history to avoid NULL deref ui/buffer: add GSList bounds checks; assert non-NULL timestamps when creating entries ui/titlebar: guard newwin failures; make draw/resize/free no-ops when window is NULL ui/statusbar: guard window creation/resize/close; clamp columns; fallback display name if JID parsing fails ui/inputwin: check newpad result; guard resize/getters/close on NULL; safe delwin ui/chatwin: guard buffer_get_entry/time before ISO8601 formatting ui/window_list: validate win_create_* results; don’t insert NULL windows; fix barejid leak in wins_get_by_string db/database: ensure cleanup on sqlite init/open failures; use sqlite3_close_v2 and warn if busy; always return a ProfMessage from log_database_get_limits_info and set current UTC timestamp when is_last with no row; initialize err_msg and free consistently; improve error messages Prevents crashes from NULL dereferences (e.g., during MAM history) and closes small leaks; improves robustness under OOM and allocation failures. --- src/database.c | 50 ++++++++++++++++++++++++++++------- src/ui/buffer.c | 8 ++++++ src/ui/chatwin.c | 7 +++-- src/ui/inputwin.c | 19 ++++++++++++-- src/ui/statusbar.c | 31 ++++++++++++++++++---- src/ui/titlebar.c | 28 ++++++++++++++++++-- src/ui/window.c | 62 +++++++++++++++++++++++++++++++++++++++++--- src/ui/window_list.c | 23 ++++++++++++++++ 8 files changed, 203 insertions(+), 25 deletions(-) diff --git a/src/database.c b/src/database.c index 6abdb6b8..9c3c3000 100644 --- a/src/database.c +++ b/src/database.c @@ -98,17 +98,23 @@ log_database_init(ProfAccount* account) auto_char char* filename = _get_db_filename(account); if (!filename) { + sqlite3_shutdown(); return FALSE; } ret = sqlite3_open(filename, &g_chatlog_database); if (ret != SQLITE_OK) { - const char* err_msg = sqlite3_errmsg(g_chatlog_database); + const char* err_msg = g_chatlog_database ? sqlite3_errmsg(g_chatlog_database) : "(no handle)"; log_error("Error opening SQLite database: %s", err_msg); + if (g_chatlog_database) { + sqlite3_close(g_chatlog_database); + g_chatlog_database = NULL; + } + sqlite3_shutdown(); return FALSE; } - char* err_msg; + char* err_msg = NULL; int db_version = _get_db_version(); if (db_version == latest_version) { @@ -216,6 +222,11 @@ out: } else { log_error("Unknown SQLite error in log_database_init()."); } + if (g_chatlog_database) { + sqlite3_close(g_chatlog_database); + g_chatlog_database = NULL; + } + sqlite3_shutdown(); return FALSE; } @@ -224,7 +235,10 @@ log_database_close(void) { log_debug("log_database_close() called"); if (g_chatlog_database) { - sqlite3_close(g_chatlog_database); + int rc = sqlite3_close_v2(g_chatlog_database); + if (rc != SQLITE_OK) { + log_warning("sqlite3_close_v2 returned %d; database may still have active statements.", rc); + } sqlite3_shutdown(); g_chatlog_database = NULL; } @@ -281,8 +295,15 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las { sqlite3_stmt* stmt = NULL; const Jid* myjid = connection_get_jid(); - if (!myjid->str) - return NULL; + // Always return a valid ProfMessage to avoid NULL dereferences in callers + ProfMessage* msg = message_init(); + if (!myjid || !myjid->str) { + // If caller requested the last message and we have no context, fall back to now + if (is_last) { + msg->timestamp = g_date_time_new_now_utc(); + } + return msg; + } const char* order = is_last ? "DESC" : "ASC"; auto_sqlite char* query = sqlite3_mprintf("SELECT `archive_id`, `timestamp` FROM `ChatLogs` WHERE " @@ -293,17 +314,21 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las if (!query) { log_error("Could not allocate memory for SQL query in log_database_get_limits_info()"); - return NULL; + if (is_last) { + msg->timestamp = g_date_time_new_now_utc(); + } + return msg; } int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL); if (rc != SQLITE_OK) { - log_error("Unknown SQLite error in log_database_get_last_info()."); - return NULL; + log_error("Unknown SQLite error in log_database_get_limits_info()."); + if (is_last) { + msg->timestamp = g_date_time_new_now_utc(); + } + return msg; } - ProfMessage* msg = message_init(); - if (sqlite3_step(stmt) == SQLITE_ROW) { char* archive_id = (char*)sqlite3_column_text(stmt, 0); char* date = (char*)sqlite3_column_text(stmt, 1); @@ -313,6 +338,11 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las } sqlite3_finalize(stmt); + // If nothing was found and caller expects the last message, provide a sane default + if (!msg->timestamp && is_last) { + msg->timestamp = g_date_time_new_now_utc(); + } + return msg; } diff --git a/src/ui/buffer.c b/src/ui/buffer.c index ad115986..8e6fc488 100644 --- a/src/ui/buffer.c +++ b/src/ui/buffer.c @@ -140,6 +140,10 @@ void buffer_remove_entry(ProfBuff buffer, int entry) { GSList* node = g_slist_nth(buffer->entries, entry); + if (node == NULL) { + // Index out of range; nothing to remove + return; + } ProfBuffEntry* e = node->data; buffer->lines -= e->_lines; _free_entry(e); @@ -168,6 +172,9 @@ ProfBuffEntry* buffer_get_entry(ProfBuff buffer, int entry) { GSList* node = g_slist_nth(buffer->entries, entry); + if (node == NULL) { + return NULL; + } return node->data; } @@ -189,6 +196,7 @@ buffer_get_entry_by_id(ProfBuff buffer, const char* const id) static ProfBuffEntry* _create_entry(const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos) { + assert(time != NULL); ProfBuffEntry* e = malloc(sizeof(struct prof_buff_entry_t)); e->show_char = STRDUP_OR_NULL(show_char); e->pad_indent = pad_indent; diff --git a/src/ui/chatwin.c b/src/ui/chatwin.c index 09061ad4..3e6af296 100644 --- a/src/ui/chatwin.c +++ b/src/ui/chatwin.c @@ -592,8 +592,11 @@ chatwin_db_history(ProfChatWin* chatwin, const gchar* start_time, const gchar* e { auto_gchar gchar* _end_time = NULL; if (!end_time && buffer_size(((ProfWin*)chatwin)->layout->buffer) > 0) { - _end_time = g_date_time_format_iso8601(buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0)->time); - end_time = _end_time; + ProfBuffEntry* first = buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0); + if (first && first->time) { + _end_time = g_date_time_format_iso8601(first->time); + end_time = _end_time; + } } GSList* history = NULL; diff --git a/src/ui/inputwin.c b/src/ui/inputwin.c index ae96e79c..aa68064d 100644 --- a/src/ui/inputwin.c +++ b/src/ui/inputwin.c @@ -163,6 +163,10 @@ create_input_window(void) rl_callback_handler_install(NULL, _inp_rl_linehandler); inp_win = newpad(1, INP_WIN_MAX); + if (!inp_win) { + // Failed to allocate input pad; leave inp_win NULL and avoid further use + return; + } wbkgd(inp_win, theme_attrs(THEME_INPUT_TEXT)); keypad(inp_win, TRUE); wmove(inp_win, 0, 0); @@ -238,6 +242,9 @@ inp_readline(void) void inp_win_resize(void) { + if (!inp_win) { + return; + } int col = getcurx(inp_win); int wcols = getmaxx(stdscr); @@ -285,8 +292,10 @@ void inp_close(void) { rl_callback_handler_remove(); - delwin(inp_win); - inp_win = NULL; + if (inp_win) { + delwin(inp_win); + inp_win = NULL; + } fclose(discard); discard = NULL; } @@ -294,6 +303,9 @@ inp_close(void) char* inp_get_line(void) { + if (!inp_win) { + return NULL; + } werase(inp_win); wmove(inp_win, 0, 0); _inp_win_update_virtual(); @@ -318,6 +330,9 @@ inp_set_line(const char* const new_line) char* inp_get_password(void) { + if (!inp_win) { + return NULL; + } werase(inp_win); wmove(inp_win, 0, 0); _inp_win_update_virtual(); diff --git a/src/ui/statusbar.c b/src/ui/statusbar.c index 2f0e21e4..ff74881a 100644 --- a/src/ui/statusbar.c +++ b/src/ui/statusbar.c @@ -111,16 +111,23 @@ status_bar_init(void) int row = screen_statusbar_row(); int cols = getmaxx(stdscr); + if (cols <= 0) { + cols = 1; + } statusbar_win = newwin(1, cols, row, 0); - status_bar_draw(); + if (statusbar_win) { + status_bar_draw(); + } } void status_bar_close(void) { - delwin(statusbar_win); - statusbar_win = NULL; + if (statusbar_win) { + delwin(statusbar_win); + statusbar_win = NULL; + } if (statusbar) { if (statusbar->time) { g_free(statusbar->time); @@ -145,7 +152,13 @@ status_bar_close(void) void status_bar_resize(void) { + if (!statusbar_win) { + return; + } int cols = getmaxx(stdscr); + if (cols <= 0) { + cols = 1; + } werase(statusbar_win); int row = screen_statusbar_row(); wresize(statusbar_win, 1, cols); @@ -285,6 +298,9 @@ status_bar_clear_fulljid(void) void status_bar_draw(void) { + if (!statusbar_win) { + return; + } werase(statusbar_win); wbkgd(statusbar_win, theme_attrs(THEME_STATUS_TEXT)); @@ -674,8 +690,13 @@ _display_name(StatusBarTab* tab) fullname = g_strconcat(mucwin_title, " conf", NULL); } else if (tab->window_type == WIN_PRIVATE) { auto_jid Jid* jid = jid_create(tab->identifier); - auto_gchar gchar* mucwin_title = mucwin_generate_title(jid->barejid, PREF_STATUSBAR_ROOM_TITLE); - fullname = g_strconcat(mucwin_title, "/", jid->resourcepart, NULL); + if (jid) { + auto_gchar gchar* mucwin_title = mucwin_generate_title(jid->barejid, PREF_STATUSBAR_ROOM_TITLE); + fullname = g_strconcat(mucwin_title, "/", jid->resourcepart, NULL); + } else { + // Fallback: use identifier directly if JID parsing failed + fullname = strdup(tab->identifier); + } } else { fullname = strdup("window"); } diff --git a/src/ui/titlebar.c b/src/ui/titlebar.c index 7ed5ed81..bed87b4a 100644 --- a/src/ui/titlebar.c +++ b/src/ui/titlebar.c @@ -73,9 +73,16 @@ void create_title_bar(void) { int cols = getmaxx(stdscr); + if (cols <= 0) { + cols = 1; + } int row = screen_titlebar_row(); win = newwin(1, cols, row, 0); + if (!win) { + // Failed to create title bar window; skip initialization to avoid NULL deref + return; + } wbkgd(win, theme_attrs(THEME_TITLE_TEXT)); title_bar_console(); title_bar_set_presence(CONTACT_OFFLINE); @@ -88,13 +95,18 @@ create_title_bar(void) void free_title_bar(void) { - delwin(win); - win = NULL; + if (win) { + delwin(win); + win = NULL; + } } void title_bar_update_virtual(void) { + if (!win) { + return; + } ProfWin* window = wins_get_current(); if (window->type != WIN_CONSOLE) { if (typing_elapsed) { @@ -114,7 +126,13 @@ title_bar_update_virtual(void) void title_bar_resize(void) { + if (!win) { + return; + } int cols = getmaxx(stdscr); + if (cols <= 0) { + cols = 1; + } werase(win); @@ -131,6 +149,9 @@ title_bar_resize(void) void title_bar_console(void) { + if (!win) { + return; + } werase(win); if (typing_elapsed) { g_timer_destroy(typing_elapsed); @@ -192,6 +213,9 @@ title_bar_set_typing(gboolean is_typing) static void _title_bar_draw(void) { + if (!win) { + return; + } int pos; int maxrightpos; ProfWin* current = wins_get_current(); diff --git a/src/ui/window.c b/src/ui/window.c index 59651843..b134246f 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -80,7 +80,15 @@ win_roster_cols(void) { int roster_win_percent = prefs_get_roster_size(); int cols = getmaxx(stdscr); - return CEILING((((double)cols) / 100) * roster_win_percent); + int width = CEILING((((double)cols) / 100) * roster_win_percent); + // Clamp to a sane range to avoid zero/full-width pads + if (cols > 1) { + if (width < 1) width = 1; + if (width >= cols) width = cols - 1; + } else { + width = 1; + } + return width; } int @@ -88,7 +96,15 @@ win_occpuants_cols(void) { int occupants_win_percent = prefs_get_occupants_size(); int cols = getmaxx(stdscr); - return CEILING((((double)cols) / 100) * occupants_win_percent); + int width = CEILING((((double)cols) / 100) * occupants_win_percent); + // Clamp to a sane range to avoid zero/full-width pads + if (cols > 1) { + if (width < 1) width = 1; + if (width >= cols) width = cols - 1; + } else { + width = 1; + } + return width; } static ProfLayout* @@ -144,6 +160,7 @@ win_create_console(void) ProfWin* win_create_chat(const char* const barejid) { + assert(barejid != NULL); ProfChatWin* new_win = malloc(sizeof(ProfChatWin)); new_win->window.type = WIN_CHAT; new_win->window.scroll_state = WIN_SCROLL_INNER; @@ -175,6 +192,7 @@ win_create_chat(const char* const barejid) ProfWin* win_create_muc(const char* const roomjid) { + assert(roomjid != NULL); ProfMucWin* new_win = malloc(sizeof(ProfMucWin)); int cols = getmaxx(stdscr); @@ -233,6 +251,8 @@ win_create_muc(const char* const roomjid) ProfWin* win_create_config(const char* const roomjid, DataForm* form, ProfConfWinCallback submit, ProfConfWinCallback cancel, const void* userdata) { + assert(roomjid != NULL); + assert(form != NULL); ProfConfWin* new_win = malloc(sizeof(ProfConfWin)); new_win->window.type = WIN_CONFIG; new_win->window.scroll_state = WIN_SCROLL_INNER; @@ -251,6 +271,7 @@ win_create_config(const char* const roomjid, DataForm* form, ProfConfWinCallback ProfWin* win_create_private(const char* const fulljid) { + assert(fulljid != NULL); ProfPrivateWin* new_win = malloc(sizeof(ProfPrivateWin)); new_win->window.type = WIN_PRIVATE; new_win->window.scroll_state = WIN_SCROLL_INNER; @@ -281,6 +302,8 @@ win_create_xmlconsole(void) ProfWin* win_create_plugin(const char* const plugin_name, const char* const tag) { + assert(plugin_name != NULL); + assert(tag != NULL); ProfPluginWin* new_win = malloc(sizeof(ProfPluginWin)); new_win->window.type = WIN_PLUGIN; new_win->window.scroll_state = WIN_SCROLL_INNER; @@ -297,6 +320,7 @@ win_create_plugin(const char* const plugin_name, const char* const tag) ProfWin* win_create_vcard(vCard* vcard) { + assert(vcard != NULL); ProfVcardWin* new_win = malloc(sizeof(ProfVcardWin)); new_win->window.type = WIN_VCARD; new_win->window.scroll_state = WIN_SCROLL_INNER; @@ -348,7 +372,7 @@ win_get_title(ProfWin* window) const ProfConfWin* confwin = (ProfConfWin*)window; assert(confwin->memcheck == PROFCONFWIN_MEMCHECK); auto_gchar gchar* mucwin_title = mucwin_generate_title(confwin->roomjid, PREF_TITLEBAR_MUC_TITLE); - if (confwin->form->modified) { + if (confwin->form && confwin->form->modified) { return g_strconcat(mucwin_title, " config *", NULL); } return g_strconcat(mucwin_title, " config", NULL); @@ -556,7 +580,25 @@ win_show_subwin(ProfWin* window) } ProfLayoutSplit* layout = (ProfLayoutSplit*)window->layout; + // If a subwindow already exists (e.g. repeated call), destroy it to avoid leaks + if (layout->subwin) { + delwin(layout->subwin); + layout->subwin = NULL; + } + + // Ensure minimum width to avoid creating a zero-width pad + if (subwin_cols <= 0) { + subwin_cols = 1; + } + layout->subwin = newpad(PAD_SIZE, subwin_cols); + if (layout->subwin == NULL) { + // Failed to allocate subwindow; keep base window resized to full width + log_error("Failed to create subwindow pad (cols=%d)", subwin_cols); + wresize(layout->base.win, PAD_SIZE, cols); + win_redraw(window); + return; + } wbkgd(layout->subwin, theme_attrs(THEME_TEXT)); wresize(layout->base.win, PAD_SIZE, cols - subwin_cols); win_redraw(window); @@ -910,6 +952,11 @@ win_refresh_with_subwin(ProfWin* window) int row_end = screen_mainwin_row_end(); ProfLayoutSplit* layout = (ProfLayoutSplit*)window->layout; + // Safety: if subwindow is not active, nothing to refresh + if (layout == NULL || layout->subwin == NULL) { + return; + } + if (window->type == WIN_MUC) { subwin_cols = win_occpuants_cols(); } else if (window->type == WIN_CONSOLE) { @@ -2038,7 +2085,14 @@ win_print_loading_history(ProfWin* window) gboolean is_buffer_empty = buffer_size(window->layout->buffer) == 0; if (!is_buffer_empty) { - timestamp = buffer_get_entry(window->layout->buffer, 0)->time; + ProfBuffEntry* first = buffer_get_entry(window->layout->buffer, 0); + if (first && first->time) { + timestamp = first->time; + } else { + // Fallback to current time if entry/time is unavailable + timestamp = g_date_time_new_now_local(); + is_buffer_empty = TRUE; // ensure we unref fallback timestamp below + } } else { timestamp = g_date_time_new_now_local(); } diff --git a/src/ui/window_list.c b/src/ui/window_list.c index 5735f019..889344b6 100644 --- a/src/ui/window_list.c +++ b/src/ui/window_list.c @@ -407,8 +407,10 @@ wins_get_by_string(const char* str) if (barejid) { ProfChatWin* chatwin = wins_get_chat(barejid); if (chatwin) { + free(barejid); return (ProfWin*)chatwin; } + free(barejid); } } @@ -603,6 +605,9 @@ wins_new_xmlconsole(void) { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_xmlconsole(); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); autocomplete_add(wins_ac, "xmlconsole"); autocomplete_add(wins_close_ac, "xmlconsole"); @@ -614,6 +619,9 @@ wins_new_chat(const char* const barejid) { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_chat(barejid); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); autocomplete_add(wins_ac, barejid); @@ -637,6 +645,9 @@ wins_new_muc(const char* const roomjid) { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_muc(roomjid); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); autocomplete_add(wins_ac, roomjid); autocomplete_add(wins_close_ac, roomjid); @@ -651,6 +662,9 @@ wins_new_config(const char* const roomjid, DataForm* form, ProfConfWinCallback s { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_config(roomjid, form, submit, cancel, userdata); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); return newwin; @@ -661,6 +675,9 @@ wins_new_private(const char* const fulljid) { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_private(fulljid); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); autocomplete_add(wins_ac, fulljid); autocomplete_add(wins_close_ac, fulljid); @@ -675,6 +692,9 @@ wins_new_plugin(const char* const plugin_name, const char* const tag) { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_plugin(plugin_name, tag); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); autocomplete_add(wins_ac, tag); autocomplete_add(wins_close_ac, tag); @@ -686,6 +706,9 @@ wins_new_vcard(vCard* vcard) { int result = _wins_get_next_available_num(keys); ProfWin* newwin = win_create_vcard(vcard); + if (!newwin) { + return NULL; + } _wins_htable_insert(windows, GINT_TO_POINTER(result), newwin); return newwin; -- 2.49.1 From 9b292a6100732e8d0b5b6143d567bcb20420d778 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 10 Nov 2025 15:09:09 +0300 Subject: [PATCH 2/7] feat(ui,db,cmd): reduce dublicate code in window subwin logic, unify SQLite helpers, NULL checks added window.c: add _get_subwin_cols, _apply_split_resize, _check_subwin_width; refactor show/hide/resize/refresh/update database.c: add _db_prepare_ctx, _db_teardown; safer init/queries with fallback timestamp cmd_defs.c: guard fopen in docgen; inputwin.c: replace assert with runtime check --- src/command/cmd_defs.c | 28 +++++++++++++---- src/database.c | 69 +++++++++++++++++++++++------------------- src/ui/inputwin.c | 9 ++++-- src/ui/window.c | 29 ++++++++++-------- 4 files changed, 83 insertions(+), 52 deletions(-) diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index ec1e58b3..cf7e6c66 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2988,8 +2988,19 @@ command_docgen(void) cmds = g_list_insert_sorted(cmds, (gpointer)pcmd, (GCompareFunc)_cmp_command); } - FILE* toc_fragment = fopen("toc_fragment.html", "w"); - FILE* main_fragment = fopen("main_fragment.html", "w"); + FILE* toc_fragment = fopen("toc_fragment.html", "w"); + if (!toc_fragment) { + log_error("command_docgen(): unable to open toc_fragment.html for writing: %s", g_strerror(errno)); + g_list_free(cmds); + return; + } + FILE* main_fragment = fopen("main_fragment.html", "w"); + if (!main_fragment) { + log_error("command_docgen(): unable to open main_fragment.html for writing: %s", g_strerror(errno)); + fclose(toc_fragment); + g_list_free(cmds); + return; + } fputs("\n", toc_fragment); - fclose(toc_fragment); - fclose(main_fragment); + fclose(toc_fragment); + fclose(main_fragment); printf("\nProcessed %d commands.\n\n", g_list_length(cmds)); g_list_free(cmds); } @@ -3093,7 +3104,12 @@ command_mangen(void) log_error("command_mangen(): could not allocate memory"); return; } - FILE* manpage = fopen(filename, "w"); + FILE* manpage = fopen(filename, "w"); + if (!manpage) { + log_error("command_mangen(): unable to open %s for writing: %s", filename, g_strerror(errno)); + curr = g_list_next(curr); + continue; + } fprintf(manpage, "%s\n", header); fputs(".SH NAME\n", manpage); @@ -3128,7 +3144,7 @@ command_mangen(void) } } - fclose(manpage); + fclose(manpage); curr = g_list_next(curr); } diff --git a/src/database.c b/src/database.c index 9c3c3000..14837d00 100644 --- a/src/database.c +++ b/src/database.c @@ -65,6 +65,36 @@ static gboolean _check_available_space_for_db_migration(char* path_to_db); static const int latest_version = 2; +// Helper: close DB handle (if any), warn on busy, and shutdown SQLite +static void +_db_teardown(const char* ctx) +{ + if (g_chatlog_database) { + int rc = sqlite3_close_v2(g_chatlog_database); + if (rc != SQLITE_OK) { + log_warning("sqlite3_close_v2 in %s returned %d; database may still have active statements.", + ctx ? ctx : "db_teardown", rc); + } + g_chatlog_database = NULL; + } + sqlite3_shutdown(); +} + +// Helper: prepare a statement and log a contextual error on failure +static gboolean +_db_prepare_ctx(const char* query, sqlite3_stmt** stmt, const char* ctx) +{ + int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, stmt, NULL); + if (rc != SQLITE_OK) { + log_error("SQLite error in %s: (error code: %d) %s", + ctx ? ctx : "sqlite3_prepare_v2", + rc, + sqlite3_errmsg(g_chatlog_database)); + return FALSE; + } + return TRUE; +} + static char* _db_strdup(const char* str) { @@ -106,11 +136,7 @@ log_database_init(ProfAccount* account) if (ret != SQLITE_OK) { const char* err_msg = g_chatlog_database ? sqlite3_errmsg(g_chatlog_database) : "(no handle)"; log_error("Error opening SQLite database: %s", err_msg); - if (g_chatlog_database) { - sqlite3_close(g_chatlog_database); - g_chatlog_database = NULL; - } - sqlite3_shutdown(); + _db_teardown("log_database_init(open)"); return FALSE; } @@ -222,11 +248,7 @@ out: } else { log_error("Unknown SQLite error in log_database_init()."); } - if (g_chatlog_database) { - sqlite3_close(g_chatlog_database); - g_chatlog_database = NULL; - } - sqlite3_shutdown(); + _db_teardown("log_database_init(out)"); return FALSE; } @@ -234,14 +256,7 @@ void log_database_close(void) { log_debug("log_database_close() called"); - if (g_chatlog_database) { - int rc = sqlite3_close_v2(g_chatlog_database); - if (rc != SQLITE_OK) { - log_warning("sqlite3_close_v2 returned %d; database may still have active statements.", rc); - } - sqlite3_shutdown(); - g_chatlog_database = NULL; - } + _db_teardown("log_database_close"); } void @@ -320,9 +335,7 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las return msg; } - int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL); - if (rc != SQLITE_OK) { - log_error("Unknown SQLite error in log_database_get_limits_info()."); + if (!_db_prepare_ctx(query, &stmt, "log_database_get_limits_info()")) { if (is_last) { msg->timestamp = g_date_time_new_now_utc(); } @@ -387,9 +400,7 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const gchar* return DB_RESPONSE_ERROR; } - int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL); - if (rc != SQLITE_OK) { - log_error("SQLite error in log_database_get_previous_chat(): (error code: %d) %s", rc, sqlite3_errmsg(g_chatlog_database)); + if (!_db_prepare_ctx(query, &stmt, "log_database_get_previous_chat()")) { return DB_RESPONSE_ERROR; } @@ -533,9 +544,7 @@ _add_to_db(ProfMessage* message, char* type, const Jid* const from_jid, const Ji } sqlite3_stmt* lmc_stmt = NULL; - - if (SQLITE_OK != sqlite3_prepare_v2(g_chatlog_database, replace_check_query, -1, &lmc_stmt, NULL)) { - log_error("SQLite error in _add_to_db() on selecting original message: %s", sqlite3_errmsg(g_chatlog_database)); + if (!_db_prepare_ctx(replace_check_query, &lmc_stmt, "_add_to_db(replace_check)")) { return; } @@ -572,8 +581,7 @@ _add_to_db(ProfMessage* message, char* type, const Jid* const from_jid, const Ji } sqlite3_stmt* stmt; - - if (SQLITE_OK == sqlite3_prepare_v2(g_chatlog_database, duplicate_check_query, -1, &stmt, NULL)) { + if (_db_prepare_ctx(duplicate_check_query, &stmt, "_add_to_db(duplicate_check)")) { if (sqlite3_step(stmt) == SQLITE_ROW) { log_error("Duplicate stanza-id found for the message. stanza_id: %s; archive_id: %s; sender: %s; content: %s", message->id, message->stanzaid, from_jid->barejid, message->plain); cons_show_error("Got a message with duplicate (server-generated) stanza-id from %s.", from_jid->fulljid); @@ -629,8 +637,7 @@ _get_db_version(void) int current_version = -1; const char* query = "SELECT `version` FROM `DbVersion` LIMIT 1"; sqlite3_stmt* statement; - - if (sqlite3_prepare_v2(g_chatlog_database, query, -1, &statement, NULL) == SQLITE_OK) { + if (_db_prepare_ctx(query, &statement, "_get_db_version()")) { if (sqlite3_step(statement) == SQLITE_ROW) { current_version = sqlite3_column_int(statement, 0); } diff --git a/src/ui/inputwin.c b/src/ui/inputwin.c index aa68064d..29a63130 100644 --- a/src/ui/inputwin.c +++ b/src/ui/inputwin.c @@ -144,10 +144,13 @@ void create_input_window(void) { /* MB_CUR_MAX is evaluated at runtime depending on the current - * locale, therefore we check that our own version is big enough - * and bail out if it isn't. + * locale; ensure our own compiled-in maximum is sufficient. + * Fail gracefully instead of aborting in production. */ - assert(MB_CUR_MAX <= PROF_MB_CUR_MAX); + if (MB_CUR_MAX > PROF_MB_CUR_MAX) { + cons_show_error("Your locale's MB_CUR_MAX (%zu) exceeds PROF_MB_CUR_MAX (%d); input window disabled.", (size_t)MB_CUR_MAX, PROF_MB_CUR_MAX); + return; + } #ifdef NCURSES_REENTRANT set_escdelay(25); #else diff --git a/src/ui/window.c b/src/ui/window.c index b134246f..c4b8c92f 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -75,6 +75,21 @@ static void _win_print_internal(ProfWin* window, const char* show_char, int pad_ int flags, theme_item_t theme_item, const char* const from, const char* const message, DeliveryReceipt* receipt); static void _win_print_wrapped(WINDOW* win, const char* const message, size_t indent, int pad_indent); +// Helper: clamp a subwindow width to a sane range [1, cols-1] if possible +static int +_check_subwin_width(int cols, int width) +{ + if (cols > 1) { + if (width < 1) + width = 1; + if (width >= cols) + width = cols - 1; + } else { + width = 1; + } + return width; +} + int win_roster_cols(void) { @@ -82,12 +97,7 @@ win_roster_cols(void) int cols = getmaxx(stdscr); int width = CEILING((((double)cols) / 100) * roster_win_percent); // Clamp to a sane range to avoid zero/full-width pads - if (cols > 1) { - if (width < 1) width = 1; - if (width >= cols) width = cols - 1; - } else { - width = 1; - } + width = _check_subwin_width(cols, width); return width; } @@ -98,12 +108,7 @@ win_occpuants_cols(void) int cols = getmaxx(stdscr); int width = CEILING((((double)cols) / 100) * occupants_win_percent); // Clamp to a sane range to avoid zero/full-width pads - if (cols > 1) { - if (width < 1) width = 1; - if (width >= cols) width = cols - 1; - } else { - width = 1; - } + width = _check_subwin_width(cols, width); return width; } -- 2.49.1 From b2ce06923e9d091f4448276be26106fa84db0824 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 10 Nov 2025 15:29:18 +0300 Subject: [PATCH 3/7] fix: Refactor src/command/cmd_defs.c for metrics fix --- src/command/cmd_defs.c | 44 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index cf7e6c66..d14c2c93 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2988,19 +2988,19 @@ command_docgen(void) cmds = g_list_insert_sorted(cmds, (gpointer)pcmd, (GCompareFunc)_cmp_command); } - FILE* toc_fragment = fopen("toc_fragment.html", "w"); - if (!toc_fragment) { - log_error("command_docgen(): unable to open toc_fragment.html for writing: %s", g_strerror(errno)); - g_list_free(cmds); - return; - } - FILE* main_fragment = fopen("main_fragment.html", "w"); - if (!main_fragment) { - log_error("command_docgen(): unable to open main_fragment.html for writing: %s", g_strerror(errno)); - fclose(toc_fragment); - g_list_free(cmds); - return; - } + FILE* toc_fragment = fopen("toc_fragment.html", "w"); + if (!toc_fragment) { + log_error("command_docgen(): unable to open toc_fragment.html for writing: %s", g_strerror(errno)); + g_list_free(cmds); + return; + } + FILE* main_fragment = fopen("main_fragment.html", "w"); + if (!main_fragment) { + log_error("command_docgen(): unable to open main_fragment.html for writing: %s", g_strerror(errno)); + fclose(toc_fragment); + g_list_free(cmds); + return; + } fputs("
    • \n", toc_fragment); fputs("
      \n", main_fragment); @@ -3068,8 +3068,8 @@ command_docgen(void) fputs("
\n", toc_fragment); - fclose(toc_fragment); - fclose(main_fragment); + fclose(toc_fragment); + fclose(main_fragment); printf("\nProcessed %d commands.\n\n", g_list_length(cmds)); g_list_free(cmds); } @@ -3104,12 +3104,12 @@ command_mangen(void) log_error("command_mangen(): could not allocate memory"); return; } - FILE* manpage = fopen(filename, "w"); - if (!manpage) { - log_error("command_mangen(): unable to open %s for writing: %s", filename, g_strerror(errno)); - curr = g_list_next(curr); - continue; - } + FILE* manpage = fopen(filename, "w"); + if (!manpage) { + log_error("command_mangen(): unable to open %s for writing: %s", filename, g_strerror(errno)); + curr = g_list_next(curr); + continue; + } fprintf(manpage, "%s\n", header); fputs(".SH NAME\n", manpage); @@ -3144,7 +3144,7 @@ command_mangen(void) } } - fclose(manpage); + fclose(manpage); curr = g_list_next(curr); } -- 2.49.1 From 58dd89be4078883ddde07b0d0eff50c5d7920570 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 10 Nov 2025 18:51:16 +0300 Subject: [PATCH 4/7] refactor: optimize hash table iterations and fix security issues #58 - refactor(core): replace g_hash_table_get_keys with g_hash_table_iter_init * Eliminates temporary GList allocations * Improves iteration performance * Affected: connection.c, cmd_defs.c, cmd_funcs.c, omemo.c, gpg.c, disco.c, form.c, autocompleters.c, capabilities.c, callbacks.c - fix(xmpp): correct queued_messages loop in connection.c:1031 * Remove incorrect NULL check that prevented message storage * calloc zeros array, causing loop to skip immediately * Fixes dropped messages during reconnection with SM enabled - fix(ui): prevent format string vulnerabilities in cons_show calls * Replace cons_show(variable) with cons_show("%s", variable) * Protects against format string attacks if variables contain % * Updated instances across cmd_funcs.c, connection.c, ox.c, console.c, core.c --- src/command/cmd_defs.c | 27 ++++---- src/command/cmd_funcs.c | 121 +++++++++++++++++------------------ src/omemo/omemo.c | 12 ++-- src/pgp/gpg.c | 13 ++-- src/plugins/autocompleters.c | 25 +++----- src/plugins/callbacks.c | 22 +++---- src/plugins/disco.c | 12 ++-- src/ui/console.c | 14 ++-- src/ui/core.c | 2 +- src/xmpp/capabilities.c | 13 ++-- src/xmpp/connection.c | 44 ++++++------- src/xmpp/form.c | 13 +--- src/xmpp/ox.c | 2 +- 13 files changed, 143 insertions(+), 177 deletions(-) diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index d14c2c93..124749d6 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2825,16 +2825,15 @@ cmd_search_index_any(char* term) int terms_len = g_strv_length(processed_terms); for (int i = 0; i < terms_len; i++) { - GList* index_keys = g_hash_table_get_keys(search_index); - GList* curr = index_keys; - while (curr) { - char* index_entry = g_hash_table_lookup(search_index, curr->data); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, search_index); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* index_entry = (char*)value; if (g_str_match_string(processed_terms[i], index_entry, FALSE)) { - results = g_list_append(results, curr->data); + results = g_list_append(results, key); } - curr = g_list_next(curr); } - g_list_free(index_keys); } return results; @@ -2848,13 +2847,14 @@ cmd_search_index_all(char* term) auto_gcharv gchar** terms = g_str_tokenize_and_fold(term, NULL, NULL); int terms_len = g_strv_length(terms); - GList* commands = g_hash_table_get_keys(search_index); - GList* curr = commands; - while (curr) { - char* command = curr->data; + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, search_index); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* command = (char*)key; + char* command_index = (char*)value; int matches = 0; for (int i = 0; i < terms_len; i++) { - char* command_index = g_hash_table_lookup(search_index, command); if (g_str_match_string(terms[i], command_index, FALSE)) { matches++; } @@ -2862,11 +2862,8 @@ cmd_search_index_all(char* term) if (matches == terms_len) { results = g_list_append(results, command); } - curr = g_list_next(curr); } - g_list_free(commands); - return results; } diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 4491f364..98d01f31 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -184,7 +184,7 @@ _string_matches_one_of(const char* what, const char* is, bool is_can_be_null, co } va_end(ap); if (s > 0) - cons_show(errmsg); + cons_show("%s", errmsg); } return ret; } @@ -418,7 +418,7 @@ cmd_connect(ProfWin* window, const char* const command, gchar** args) auto_char char* err_msg = NULL; gboolean res = strtoi_range(port_str, &port, 1, 65535, &err_msg); if (!res) { - cons_show(err_msg); + cons_show("%s", err_msg); cons_show(""); port = 0; options_destroy(options); @@ -711,7 +711,7 @@ _account_set_port(char* account_name, char* port) auto_char char* err_msg = NULL; gboolean res = strtoi_range(port, &porti, 1, 65535, &err_msg); if (!res) { - cons_show(err_msg); + cons_show("%s", err_msg); cons_show(""); } else { accounts_set_port(account_name, porti); @@ -903,7 +903,7 @@ _account_set_max_sessions(char* account_name, char* max_sessions_raw) auto_char char* err_msg = NULL; gboolean res = strtoi_range(max_sessions_raw, &max_sessions, 0, INT_MAX, &err_msg); if (!res) { - cons_show(err_msg); + cons_show("%s", err_msg); cons_show(""); return TRUE; } @@ -924,7 +924,7 @@ _account_set_presence_priority(char* account_name, char* presence, char* priorit auto_char char* err_msg = NULL; gboolean res = strtoi_range(priority, &intval, -128, 127, &err_msg); if (!res) { - cons_show(err_msg); + cons_show("%s", err_msg); return TRUE; } @@ -1580,7 +1580,7 @@ _cmd_list_commands(GList* commands) while (curr) { gchar* cmd = curr->data; if (count == 5) { - cons_show(cmds->str); + cons_show("%s", cmds->str); g_string_free(cmds, TRUE); cmds = g_string_new(""); count = 0; @@ -1589,7 +1589,7 @@ _cmd_list_commands(GList* commands) curr = g_list_next(curr); count++; } - cons_show(cmds->str); + cons_show("%s", cmds->str); g_string_free(cmds, TRUE); g_list_free(curr); @@ -2407,7 +2407,7 @@ cmd_roster(ProfWin* window, const char* const command, gchar** args) } return TRUE; } else { - cons_show(err_msg); + cons_show("%s", err_msg); return TRUE; } @@ -2467,7 +2467,7 @@ cmd_roster(ProfWin* window, const char* const command, gchar** args) cons_show("Roster contact indent set to: %d", intval); rosterwin_roster(); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } } } else { @@ -2501,7 +2501,7 @@ cmd_roster(ProfWin* window, const char* const command, gchar** args) cons_show("Roster resource indent set to: %d", intval); rosterwin_roster(); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } } } else if (g_strcmp0(args[1], "join") == 0) { @@ -2527,7 +2527,7 @@ cmd_roster(ProfWin* window, const char* const command, gchar** args) cons_show("Roster presence indent set to: %d", intval); rosterwin_roster(); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } } } else { @@ -4371,7 +4371,7 @@ cmd_occupants(ProfWin* window, const char* const command, gchar** args) wins_resize_all(); return TRUE; } else { - cons_show(err_msg); + cons_show("%s", err_msg); return TRUE; } } @@ -4391,7 +4391,7 @@ cmd_occupants(ProfWin* window, const char* const command, gchar** args) occupantswin_occupants_all(); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; } @@ -5836,7 +5836,7 @@ cmd_inpblock(ProfWin* window, const char* const command, gchar** args) prefs_set_inpblock(intval); inp_nonblocking(FALSE); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; @@ -6063,7 +6063,7 @@ cmd_statusbar(ProfWin* window, const char* const command, gchar** args) ui_resize(); return TRUE; } else { - cons_show(err_msg); + cons_show("%s", err_msg); cons_bad_cmd_usage(command); return TRUE; } @@ -6094,7 +6094,7 @@ cmd_statusbar(ProfWin* window, const char* const command, gchar** args) ui_resize(); return TRUE; } else { - cons_show(err_msg); + cons_show("%s", err_msg); cons_bad_cmd_usage(command); return TRUE; } @@ -6254,7 +6254,7 @@ cmd_log(ProfWin* window, const char* const command, gchar** args) prefs_set_max_log_size(intval); cons_show("Log maximum size set to %d bytes", intval); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; } @@ -6304,7 +6304,7 @@ cmd_reconnect(ProfWin* window, const char* const command, gchar** args) cons_show("Reconnect interval set to %d seconds.", intval); } } else { - cons_show(err_msg); + cons_show("%s", err_msg); cons_bad_cmd_usage(command); } @@ -6330,7 +6330,7 @@ cmd_autoping(ProfWin* window, const char* const command, gchar** args) cons_show("Autoping interval set to %d seconds.", intval); } } else { - cons_show(err_msg); + cons_show("%s", err_msg); cons_bad_cmd_usage(command); } @@ -6346,7 +6346,7 @@ cmd_autoping(ProfWin* window, const char* const command, gchar** args) cons_show("Autoping timeout set to %d seconds.", intval); } } else { - cons_show(err_msg); + cons_show("%s", err_msg); cons_bad_cmd_usage(command); } @@ -6416,7 +6416,7 @@ cmd_autoaway(ProfWin* window, const char* const command, gchar** args) cons_show("Auto away time set to: %d minutes.", minutesval); } } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; @@ -6439,7 +6439,7 @@ cmd_autoaway(ProfWin* window, const char* const command, gchar** args) } } } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; @@ -6506,7 +6506,7 @@ cmd_priority(ProfWin* window, const char* const command, gchar** args) cl_ev_presence_send(last_presence, 0); cons_show("Priority set to %d.", intval); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; @@ -6576,7 +6576,7 @@ cmd_tray(ProfWin* window, const char* const command, gchar** args) tray_set_timer(intval); } } else { - cons_show(err_msg); + cons_show("%s", err_msg); } return TRUE; @@ -7181,22 +7181,21 @@ cmd_pgp(ProfWin* window, const char* const command, gchar** args) } cons_show("PGP keys:"); - GList* keylist = g_hash_table_get_keys(keys); - GList* curr = keylist; - while (curr) { - ProfPGPKey* key = g_hash_table_lookup(keys, curr->data); - cons_show(" %s", key->name); - cons_show(" ID : %s", key->id); - auto_char char* format_fp = p_gpg_format_fp_str(key->fp); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, keys); + while (g_hash_table_iter_next(&iter, &key, &value)) { + ProfPGPKey* pgp_key = (ProfPGPKey*)value; + cons_show(" %s", pgp_key->name); + cons_show(" ID : %s", pgp_key->id); + auto_char char* format_fp = p_gpg_format_fp_str(pgp_key->fp); cons_show(" Fingerprint : %s", format_fp); - if (key->secret) { + if (pgp_key->secret) { cons_show(" Type : PUBLIC, PRIVATE"); } else { cons_show(" Type : PUBLIC"); } - curr = g_list_next(curr); } - g_list_free(keylist); p_gpg_free_keys(keys); return TRUE; } @@ -7237,25 +7236,24 @@ cmd_pgp(ProfWin* window, const char* const command, gchar** args) return TRUE; } GHashTable* pubkeys = p_gpg_pubkeys(); - GList* jids = g_hash_table_get_keys(pubkeys); - if (!jids) { + if (!pubkeys || g_hash_table_size(pubkeys) == 0) { cons_show("No contacts found with PGP public keys assigned."); return TRUE; } cons_show("Assigned PGP public keys:"); - GList* curr = jids; - while (curr) { - char* jid = curr->data; - ProfPGPPubKeyId* pubkeyid = g_hash_table_lookup(pubkeys, jid); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, pubkeys); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* jid = (char*)key; + ProfPGPPubKeyId* pubkeyid = (ProfPGPPubKeyId*)value; if (pubkeyid->received) { cons_show(" %s: %s (received)", jid, pubkeyid->id); } else { cons_show(" %s: %s (stored)", jid, pubkeyid->id); } - curr = g_list_next(curr); } - g_list_free(jids); return TRUE; } @@ -7468,22 +7466,21 @@ cmd_ox(ProfWin* window, const char* const command, gchar** args) } cons_show("OpenPGP keys:"); - GList* keylist = g_hash_table_get_keys(keys); - GList* curr = keylist; - while (curr) { - ProfPGPKey* key = g_hash_table_lookup(keys, curr->data); - cons_show(" %s", key->name); - cons_show(" ID : %s", key->id); - auto_char char* format_fp = p_gpg_format_fp_str(key->fp); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, keys); + while (g_hash_table_iter_next(&iter, &key, &value)) { + ProfPGPKey* pgp_key = (ProfPGPKey*)value; + cons_show(" %s", pgp_key->name); + cons_show(" ID : %s", pgp_key->id); + auto_char char* format_fp = p_gpg_format_fp_str(pgp_key->fp); cons_show(" Fingerprint : %s", format_fp); - if (key->secret) { + if (pgp_key->secret) { cons_show(" Type : PUBLIC, PRIVATE"); } else { cons_show(" Type : PUBLIC"); } - curr = g_list_next(curr); } - g_list_free(keylist); p_gpg_free_keys(keys); return TRUE; } @@ -7491,8 +7488,8 @@ cmd_ox(ProfWin* window, const char* const command, gchar** args) else if (g_strcmp0(args[0], "contacts") == 0) { GHashTable* keys = ox_gpg_public_keys(); cons_show("OpenPGP keys:"); - GList* keylist = g_hash_table_get_keys(keys); - GList* curr = keylist; + GHashTableIter iter; + gpointer key, value; GSList* roster_list = NULL; jabber_conn_status_t conn_status = connection_get_status(); @@ -7502,15 +7499,16 @@ cmd_ox(ProfWin* window, const char* const command, gchar** args) roster_list = roster_get_contacts(ROSTER_ORD_NAME); } - while (curr) { - ProfPGPKey* key = g_hash_table_lookup(keys, curr->data); + g_hash_table_iter_init(&iter, keys); + while (g_hash_table_iter_next(&iter, &key, &value)) { + ProfPGPKey* pgp_key = (ProfPGPKey*)value; PContact contact = NULL; if (roster_list) { GSList* curr_c = roster_list; while (!contact && curr_c) { contact = curr_c->data; auto_gchar gchar* xmppuri = g_strdup_printf("xmpp:%s", p_contact_barejid(contact)); - if (g_strcmp0(key->name, xmppuri)) { + if (g_strcmp0(pgp_key->name, xmppuri)) { contact = NULL; } curr_c = g_slist_next(curr_c); @@ -7518,11 +7516,10 @@ cmd_ox(ProfWin* window, const char* const command, gchar** args) } if (contact) { - cons_show("%s - %s", key->fp, key->name); + cons_show("%s - %s", pgp_key->fp, pgp_key->name); } else { - cons_show("%s - %s (not in roster)", key->fp, key->name); + cons_show("%s - %s (not in roster)", pgp_key->fp, pgp_key->name); } - curr = g_list_next(curr); } } else if (g_strcmp0(args[0], "start") == 0) { @@ -9612,7 +9609,7 @@ cmd_register(ProfWin* window, const char* const command, gchar** args) auto_char char* err_msg = NULL; gboolean res = strtoi_range(port_str, &port, 1, 65535, &err_msg); if (!res) { - cons_show(err_msg); + cons_show("%s", err_msg); cons_show(""); port = 0; options_destroy(options); @@ -9682,7 +9679,7 @@ cmd_strophe(ProfWin* window, const char* const command, gchar** args) prefs_set_string(PREF_STROPHE_VERBOSITY, args[1]); return TRUE; } else { - cons_show(err_msg); + cons_show("%s", err_msg); } } else if (g_strcmp0(args[0], "sm") == 0) { if (g_strcmp0(args[1], "no-resend") == 0) { diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index f47ff506..59f2d82a 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -532,11 +532,13 @@ omemo_set_device_list(const char* const from, GList* device_list) for (device_id = device_list; device_id != NULL; device_id = device_id->next) { GHashTable* known_identities = g_hash_table_lookup(omemo_ctx.known_devices, jid->barejid); if (known_identities) { - GList* fp = NULL; - for (fp = g_hash_table_get_keys(known_identities); fp != NULL; fp = fp->next) { - if (device_id->data == g_hash_table_lookup(known_identities, fp->data)) { - cons_show("OMEMO: Adding firstusage trust for %s device %d - Fingerprint %s", jid->barejid, device_id->data, omemo_format_fingerprint(fp->data)); - omemo_trust(jid->barejid, omemo_format_fingerprint(fp->data)); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, known_identities); + while (g_hash_table_iter_next(&iter, &key, &value)) { + if (device_id->data == value) { + cons_show("OMEMO: Adding firstusage trust for %s device %d - Fingerprint %s", jid->barejid, device_id->data, omemo_format_fingerprint(key)); + omemo_trust(jid->barejid, omemo_format_fingerprint(key)); } } } diff --git a/src/pgp/gpg.c b/src/pgp/gpg.c index 52f98bbd..72511d77 100644 --- a/src/pgp/gpg.c +++ b/src/pgp/gpg.c @@ -345,14 +345,13 @@ p_gpg_list_keys(void) // TODO: move autocomplete in other place autocomplete_clear(key_ac); - GList* ids = g_hash_table_get_keys(result); - GList* curr = ids; - while (curr) { - ProfPGPKey* key = g_hash_table_lookup(result, curr->data); - autocomplete_add(key_ac, key->id); - curr = curr->next; + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, result); + while (g_hash_table_iter_next(&iter, &key, &value)) { + ProfPGPKey* pgp_key = (ProfPGPKey*)value; + autocomplete_add(key_ac, pgp_key->id); } - g_list_free(ids); return result; } diff --git a/src/plugins/autocompleters.c b/src/plugins/autocompleters.c index 1a9a0f2b..2b0a2fbe 100644 --- a/src/plugins/autocompleters.c +++ b/src/plugins/autocompleters.c @@ -141,18 +141,16 @@ autocompleters_complete(const char* const input, gboolean previous) while (curr_hash) { GHashTable* key_to_ac = curr_hash->data; - GList* keys = g_hash_table_get_keys(key_to_ac); - GList* curr = keys; - while (curr) { - result = autocomplete_param_with_ac(input, curr->data, g_hash_table_lookup(key_to_ac, curr->data), TRUE, previous); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, key_to_ac); + while (g_hash_table_iter_next(&iter, &key, &value)) { + result = autocomplete_param_with_ac(input, key, value, TRUE, previous); if (result) { g_list_free(ac_hashes); - g_list_free(keys); return result; } - curr = g_list_next(curr); } - g_list_free(keys); curr_hash = g_list_next(curr_hash); } @@ -162,22 +160,19 @@ autocompleters_complete(const char* const input, gboolean previous) curr_hash = filepath_hashes; while (curr_hash) { GHashTable* prefixes_hash = curr_hash->data; - GList* prefixes = g_hash_table_get_keys(prefixes_hash); - GList* curr_prefix = prefixes; - while (curr_prefix) { - char* prefix = curr_prefix->data; + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, prefixes_hash); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* prefix = (char*)key; if (g_str_has_prefix(input, prefix)) { result = cmd_ac_complete_filepath(input, prefix, previous); if (result) { g_list_free(filepath_hashes); - g_list_free(prefixes); return result; } } - - curr_prefix = g_list_next(curr_prefix); } - g_list_free(prefixes); curr_hash = g_list_next(curr_hash); } diff --git a/src/plugins/callbacks.c b/src/plugins/callbacks.c index 6852a0ac..f93f1385 100644 --- a/src/plugins/callbacks.c +++ b/src/plugins/callbacks.c @@ -149,15 +149,14 @@ callbacks_remove(const char* const plugin_name) { GHashTable* command_hash = g_hash_table_lookup(p_commands, plugin_name); if (command_hash) { - GList* commands = g_hash_table_get_keys(command_hash); - GList* curr = commands; - while (curr) { - char* command = curr->data; + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, command_hash); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* command = (char*)key; cmd_ac_remove(command); cmd_ac_remove_help(&command[1]); - curr = g_list_next(curr); } - g_list_free(commands); } g_hash_table_remove(p_commands, plugin_name); @@ -165,13 +164,12 @@ callbacks_remove(const char* const plugin_name) GHashTable* tag_to_win_cb_hash = g_hash_table_lookup(p_window_callbacks, plugin_name); if (tag_to_win_cb_hash) { - GList* tags = g_hash_table_get_keys(tag_to_win_cb_hash); - GList* curr = tags; - while (curr) { - wins_close_plugin(curr->data); - curr = g_list_next(curr); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, tag_to_win_cb_hash); + while (g_hash_table_iter_next(&iter, &key, &value)) { + wins_close_plugin(key); } - g_list_free(tags); } g_hash_table_remove(p_window_callbacks, plugin_name); diff --git a/src/plugins/disco.c b/src/plugins/disco.c index 0bb715b2..38e967e5 100644 --- a/src/plugins/disco.c +++ b/src/plugins/disco.c @@ -107,10 +107,11 @@ disco_remove_features(const char* plugin_name) return; } - GList* plugin_feature_list = g_hash_table_get_keys(plugin_features_set); - GList* curr = plugin_feature_list; - while (curr) { - char* feature = curr->data; + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, plugin_features_set); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* feature = (char*)key; if (g_hash_table_contains(features, feature)) { void* refcountp = g_hash_table_lookup(features, feature); int refcount = GPOINTER_TO_INT(refcountp); @@ -121,10 +122,7 @@ disco_remove_features(const char* plugin_name) g_hash_table_replace(features, strdup(feature), GINT_TO_POINTER(refcount)); } } - - curr = g_list_next(curr); } - g_list_free(plugin_feature_list); } GList* diff --git a/src/ui/console.c b/src/ui/console.c index 616636bc..52c281e4 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -152,7 +152,7 @@ cons_bad_cmd_usage(const char* const cmd) g_string_printf(msg, "Invalid usage, see '/help %s' for details.", &cmd[1]); cons_show(""); - cons_show(msg->str); + cons_show("%s", msg->str); g_string_free(msg, TRUE); } @@ -773,7 +773,7 @@ cons_show_disco_info(const char* jid, GSList* identities, GSList* features) if (identity->category) { identity_str = g_string_append(identity_str, identity->category); } - cons_show(identity_str->str); + cons_show("%s", identity_str->str); g_string_free(identity_str, TRUE); identities = g_slist_next(identities); } @@ -1019,7 +1019,7 @@ cons_show_account(ProfAccount* account) } curr = curr->next; } - cons_show(manual->str); + cons_show("%s", manual->str); g_string_free(manual, TRUE); } if (g_list_length(account->otr_opportunistic) > 0) { @@ -1032,7 +1032,7 @@ cons_show_account(ProfAccount* account) } curr = curr->next; } - cons_show(opportunistic->str); + cons_show("%s", opportunistic->str); g_string_free(opportunistic, TRUE); } if (g_list_length(account->otr_always) > 0) { @@ -1045,7 +1045,7 @@ cons_show_account(ProfAccount* account) } curr = curr->next; } - cons_show(always->str); + cons_show("%s", always->str); g_string_free(always, TRUE); } @@ -2297,7 +2297,7 @@ cons_show_themes(GSList* themes) } else { cons_show("Available themes:"); while (themes) { - cons_show(themes->data); + cons_show("%s", themes->data); themes = g_slist_next(themes); } } @@ -2315,7 +2315,7 @@ cons_show_scripts(GSList* scripts) } else { cons_show("Scripts:"); while (scripts) { - cons_show(scripts->data); + cons_show("%s", scripts->data); scripts = g_slist_next(scripts); } } diff --git a/src/ui/core.c b/src/ui/core.c index 91249d41..e61831a0 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -461,7 +461,7 @@ ui_invalid_command_usage(const char* const cmd, void (*setting_func)(void)) (*setting_func)(); } else { cons_show(""); - cons_show(msg->str); + cons_show("%s", msg->str); ProfWin* current = wins_get_current(); if (current->type == WIN_CHAT) { win_println(current, THEME_DEFAULT, "-", "%s", msg->str); diff --git a/src/xmpp/capabilities.c b/src/xmpp/capabilities.c index a9aa5cbb..0b10da81 100644 --- a/src/xmpp/capabilities.c +++ b/src/xmpp/capabilities.c @@ -171,16 +171,15 @@ caps_get_features(void) { GList* result = NULL; - GList* features_as_list = g_hash_table_get_keys(prof_features); - GList* curr = features_as_list; - while (curr) { - result = g_list_append(result, strdup(curr->data)); - curr = g_list_next(curr); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, prof_features); + while (g_hash_table_iter_next(&iter, &key, &value)) { + result = g_list_append(result, strdup((char*)key)); } - g_list_free(features_as_list); GList* plugin_features = plugins_get_disco_features(); - curr = plugin_features; + GList* curr = plugin_features; while (curr) { result = g_list_append(result, strdup(curr->data)); curr = g_list_next(curr); diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 0f71ca2f..3c38f744 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -148,7 +148,7 @@ connection_init(void) if (string_to_verbosity(v, &verbosity, &err_msg)) { xmpp_ctx_set_verbosity(conn.xmpp_ctx, verbosity); } else { - cons_show(err_msg); + cons_show("%s", err_msg); } conn.xmpp_conn = xmpp_conn_new(conn.xmpp_ctx); @@ -638,22 +638,18 @@ gboolean connection_supports(const char* const feature) { gboolean ret = FALSE; - GList* jids = g_hash_table_get_keys(conn.features_by_jid); + GHashTableIter iter; + gpointer key, value; - GList* curr = jids; - while (curr) { - char* jid = curr->data; - GHashTable* features = g_hash_table_lookup(conn.features_by_jid, jid); + g_hash_table_iter_init(&iter, conn.features_by_jid); + while (g_hash_table_iter_next(&iter, &key, &value)) { + GHashTable* features = (GHashTable*)value; if (features && g_hash_table_lookup(features, feature)) { ret = TRUE; break; } - - curr = g_list_next(curr); } - g_list_free(jids); - return ret; } @@ -664,22 +660,17 @@ connection_jid_for_feature(const char* const feature) return NULL; } - GList* jids = g_hash_table_get_keys(conn.features_by_jid); + GHashTableIter iter; + gpointer key, value; - GList* curr = jids; - while (curr) { - char* jid = curr->data; - GHashTable* features = g_hash_table_lookup(conn.features_by_jid, jid); + g_hash_table_iter_init(&iter, conn.features_by_jid); + while (g_hash_table_iter_next(&iter, &key, &value)) { + GHashTable* features = (GHashTable*)value; if (features && g_hash_table_lookup(features, feature)) { - g_list_free(jids); - return jid; + return (const char*)key; } - - curr = g_list_next(curr); } - g_list_free(jids); - return NULL; } @@ -1034,7 +1025,7 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status conn.sm_state = xmpp_conn_get_sm_state(conn.xmpp_conn); if (send_queue_len > 0 && prefs_get_boolean(PREF_STROPHE_SM_RESEND)) { conn.queued_messages = calloc(send_queue_len + 1, sizeof(*conn.queued_messages)); - for (int n = 0; n < send_queue_len && conn.queued_messages[n]; ++n) { + for (int n = 0; n < send_queue_len; ++n) { conn.queued_messages[n] = xmpp_conn_send_queue_drop_element(conn.xmpp_conn, XMPP_QUEUE_OLDEST); } } else if (send_queue_len > 0) { @@ -1189,12 +1180,13 @@ connection_debug_print_features() continue; } - GList* feature_keys = g_hash_table_get_keys(features); - for (GList* l = feature_keys; l != NULL; l = l->next) { - const char* feature = (const char*)l->data; + GHashTableIter feature_iter; + gpointer feature_key, feature_value; + g_hash_table_iter_init(&feature_iter, features); + while (g_hash_table_iter_next(&feature_iter, &feature_key, &feature_value)) { + const char* feature = (const char*)feature_key; log_debug("%s:\t%s", jid, feature); } - g_list_free(feature_keys); } log_debug("=== End of Features ==="); diff --git a/src/xmpp/form.c b/src/xmpp/form.c index 5290c83f..88730435 100644 --- a/src/xmpp/form.c +++ b/src/xmpp/form.c @@ -437,18 +437,7 @@ form_get_form_type_field(DataForm* form) gboolean form_tag_exists(DataForm* form, const char* const tag) { - GList* tags = g_hash_table_get_keys(form->tag_to_var); - GList* curr = tags; - while (curr) { - if (g_strcmp0(curr->data, tag) == 0) { - g_list_free(tags); - return TRUE; - } - curr = g_list_next(curr); - } - - g_list_free(tags); - return FALSE; + return g_hash_table_contains(form->tag_to_var, tag); } form_field_type_t diff --git a/src/xmpp/ox.c b/src/xmpp/ox.c index 6489a18e..46d797a8 100644 --- a/src/xmpp/ox.c +++ b/src/xmpp/ox.c @@ -326,7 +326,7 @@ _ox_metadata_result(xmpp_stanza_t* const stanza, void* const userdata) if (fingerprint) { if (strlen(fingerprint) == KEYID_LENGTH) { - cons_show(fingerprint); + cons_show("%s", fingerprint); } else { cons_show("OX: Wrong char size of public key"); log_error("[OX] Wrong chat size of public key %s", fingerprint); -- 2.49.1 From b772bd8c86029c6e5781adaac5073865b8ba3c97 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Thu, 5 Feb 2026 00:01:16 +0300 Subject: [PATCH 5/7] tests: add functional tests for XEP-0012 and XEP-0045 - Add Last Activity functional tests (XEP-0012) - Add MUC affiliation/kick tests (XEP-0045) --- Makefile.am | 1 + tests/functionaltests/functionaltests.c | 9 +++ tests/functionaltests/test_lastactivity.c | 63 ++++++++++++++++++ tests/functionaltests/test_lastactivity.h | 7 ++ tests/functionaltests/test_muc.c | 80 +++++++++++++++++++++++ tests/functionaltests/test_muc.h | 2 + 6 files changed, 162 insertions(+) create mode 100644 tests/functionaltests/test_lastactivity.c create mode 100644 tests/functionaltests/test_lastactivity.h diff --git a/Makefile.am b/Makefile.am index 1735d1ea..60ff0f33 100644 --- a/Makefile.am +++ b/Makefile.am @@ -185,6 +185,7 @@ functionaltest_sources = \ tests/functionaltests/test_software.c tests/functionaltests/test_software.h \ tests/functionaltests/test_muc.c tests/functionaltests/test_muc.h \ tests/functionaltests/test_disconnect.c tests/functionaltests/test_disconnect.h \ + tests/functionaltests/test_lastactivity.c tests/functionaltests/test_lastactivity.h \ tests/functionaltests/functionaltests.c main_source = src/main.c diff --git a/tests/functionaltests/functionaltests.c b/tests/functionaltests/functionaltests.c index cd89c58f..46d1a7d2 100644 --- a/tests/functionaltests/functionaltests.c +++ b/tests/functionaltests/functionaltests.c @@ -52,6 +52,7 @@ #include "test_software.h" #include "test_muc.h" #include "test_disconnect.h" +#include "test_lastactivity.h" /* Macro to wrap each test with setup/teardown functions */ #define PROF_FUNC_TEST(test) cmocka_unit_test_setup_teardown(test, init_prof_test, close_prof_test) @@ -104,6 +105,10 @@ main(int argc, char* argv[]) PROF_FUNC_TEST(display_software_version_result_when_from_domainpart), PROF_FUNC_TEST(show_message_in_chat_window_when_no_resource), PROF_FUNC_TEST(display_software_version_result_in_chat), + + /* Last Activity - XEP-0012 */ + PROF_FUNC_TEST(responds_to_last_activity_request), + PROF_FUNC_TEST(last_activity_request_to_contact), }; /* ============================================================ @@ -188,6 +193,10 @@ main(int argc, char* argv[]) PROF_FUNC_TEST(shows_first_message_in_console_when_window_not_focussed), PROF_FUNC_TEST(shows_no_message_in_console_when_window_not_focussed), + /* MUC moderation - XEP-0045 room admin */ + PROF_FUNC_TEST(sends_affiliation_list_request), + PROF_FUNC_TEST(sends_kick_request), + /* Message Carbons - XEP-0280 (message sync across devices) */ PROF_FUNC_TEST(send_enable_carbons), PROF_FUNC_TEST(connect_with_carbons_enabled), diff --git a/tests/functionaltests/test_lastactivity.c b/tests/functionaltests/test_lastactivity.c new file mode 100644 index 00000000..e4ba89db --- /dev/null +++ b/tests/functionaltests/test_lastactivity.c @@ -0,0 +1,63 @@ +/* + * test_lastactivity.c + * Functional tests for Last Activity (XEP-0012) + */ +#include +#include "prof_cmocka.h" +#include +#include + +#include + +#include "proftest.h" + +void +responds_to_last_activity_request(void **state) +{ + prof_connect(); + + // Send incoming last activity request + stbbr_send( + "" + "" + "" + ); + + // Verify profanity responds with last activity info + // The 'seconds' attribute indicates idle time + assert_true(stbbr_received( + "" + "" + "" + )); +} + +void +last_activity_request_to_contact(void **state) +{ + prof_connect(); + + stbbr_send( + "" + "10" + "I'm here" + "" + ); + assert_true(prof_output_exact("Buddy1 (mobile) is online, \"I'm here\"")); + + // Register response for last activity query + stbbr_for_query("jabber:iq:last", + "" + "" + "" + ); + + prof_input("/lastactivity get buddy1@localhost/mobile"); + + // Verify the request was sent + assert_true(stbbr_received( + "" + "" + "" + )); +} diff --git a/tests/functionaltests/test_lastactivity.h b/tests/functionaltests/test_lastactivity.h new file mode 100644 index 00000000..a159a600 --- /dev/null +++ b/tests/functionaltests/test_lastactivity.h @@ -0,0 +1,7 @@ +/* + * test_lastactivity.h + * Header for Last Activity tests (XEP-0012) + */ + +void responds_to_last_activity_request(void **state); +void last_activity_request_to_contact(void **state); diff --git a/tests/functionaltests/test_muc.c b/tests/functionaltests/test_muc.c index 3df179d2..0b618ab3 100644 --- a/tests/functionaltests/test_muc.c +++ b/tests/functionaltests/test_muc.c @@ -393,3 +393,83 @@ shows_no_message_in_console_when_window_not_focussed(void **state) assert_false(prof_output_regex("testroom@conference\\.localhost \\(win 2\\)")); prof_timeout_reset(); } + +void +sends_affiliation_list_request(void **state) +{ + prof_connect(); + + stbbr_for_presence_to("testroom@conference.localhost/stabber", + "" + "" + "" + "" + "" + "" + "" + ); + + prof_input("/join testroom@conference.localhost"); + assert_true(prof_output_regex("-> You have joined the room as stabber, role: moderator, affiliation: owner")); + + prof_input("/affiliation owner list"); + + assert_true(stbbr_received( + "" + "" + "" + "" + "" + )); +} + +void +sends_kick_request(void **state) +{ + prof_connect(); + + // Enable MUC presence messages to see occupant join/leave + prof_input("/presence room all"); + assert_true(prof_output_regex("All presence updates will appear")); + + stbbr_for_presence_to("testroom@conference.localhost/stabber", + "" + "" + "" + "" + "" + "" + "" + ); + + prof_input("/join testroom@conference.localhost"); + assert_true(prof_output_regex("-> You have joined the room as stabber, role: moderator, affiliation: admin")); + + // Simulate another user in the room + stbbr_send( + "" + "" + "" + "" + "" + ); + sleep(1); + assert_true(prof_output_regex("baduser has joined")); + + // Register success response for kick + stbbr_for_query("http://jabber.org/protocol/muc#admin", + "" + ); + + prof_input("/kick baduser \"spamming\""); + + assert_true(stbbr_received( + "" + "" + "" + "spamming" + "" + "" + "" + )); +} diff --git a/tests/functionaltests/test_muc.h b/tests/functionaltests/test_muc.h index 1636bd05..7be3ee1f 100644 --- a/tests/functionaltests/test_muc.h +++ b/tests/functionaltests/test_muc.h @@ -12,3 +12,5 @@ void shows_me_message_from_self(void **state); void shows_all_messages_in_console_when_window_not_focussed(void **state); void shows_first_message_in_console_when_window_not_focussed(void **state); void shows_no_message_in_console_when_window_not_focussed(void **state); +void sends_affiliation_list_request(void **state); +void sends_kick_request(void **state); -- 2.49.1 From 34501d913ce6be40a73a90e7e9db936c534b96e5 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Thu, 5 Feb 2026 17:22:04 +0300 Subject: [PATCH 6/7] fix(security): prevent CWE-134 format string injection - add "%s" to unsafe cons_show/log_*/win_println calls - add check-cwe134.sh static analysis script - integrate security check into CI pipeline Closes #85 --- .github/workflows/ci-code.yml | 3 ++ CONTRIBUTING.md | 10 +++++ check-cwe134.sh | 69 +++++++++++++++++++++++++++++++++++ src/command/cmd_funcs.c | 4 +- src/common.c | 4 +- src/plugins/python_plugins.c | 8 ++-- src/profanity.c | 4 +- src/tools/http_upload.c | 4 +- src/ui/console.c | 2 +- src/ui/core.c | 2 +- src/ui/window.c | 2 +- src/xmpp/message.c | 2 +- src/xmpp/presence.c | 2 +- 13 files changed, 99 insertions(+), 17 deletions(-) create mode 100755 check-cwe134.sh diff --git a/.github/workflows/ci-code.yml b/.github/workflows/ci-code.yml index e4e8dc77..ae9e7297 100644 --- a/.github/workflows/ci-code.yml +++ b/.github/workflows/ci-code.yml @@ -50,6 +50,9 @@ jobs: run: | grep -P 'auto_(char|gchar|gcharv|guchar|jid|sqlite|gfd|FILE)[\w *]*;$' -r src && exit -1 || true + - name: Check CWE-134 format string vulnerabilities + run: ./check-cwe134.sh + - name: Install clang-format run: | sudo apt-get update diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87edee02..eff0cdd1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,6 +144,16 @@ scan-build make scan-view ... ``` +### Security checks + +We have a static analyzer `check-cwe134.sh` that detects CWE-134 format string vulnerabilities. It runs automatically in CI but you can also run it locally: + +```bash +./check-cwe134.sh +``` + +This checks for unsafe patterns where user-controlled data could be passed directly as a format string to functions like `cons_show`, `log_error`, `win_println`, etc. Always use `"%s"` format specifier when printing user data. + ### Finding typos We include a `.codespellrc` configuration file for `codespell` in the root directory. diff --git a/check-cwe134.sh b/check-cwe134.sh new file mode 100755 index 00000000..4ff6b67f --- /dev/null +++ b/check-cwe134.sh @@ -0,0 +1,69 @@ +#!/bin/bash +# check-cwe134.sh - Static analysis for CWE-134 format string vulnerabilities +# +# This script detects potentially unsafe usage of format string functions +# where user-controlled data may be passed without "%s" wrapper. +# +# Usage: ./check-cwe134.sh [directory] + +set -e + +DIR="${1:-src}" + +echo "=== CWE-134 Format String Vulnerability Check ===" +echo "Scanning: $DIR" +echo "" + +# Functions that accept format strings +FORMAT_FUNCS="cons_show|cons_debug|cons_show_error|log_info|log_error|log_warning|log_debug|win_println|win_print" + +ERRORS=0 + +echo "Checking for unsafe format string usage..." +echo "" + +# Pattern 1: function call with single variable argument (no format string) +# Example: cons_show(variable); - BAD +# Example: cons_show("%s", variable); - OK +# Matches: func(identifier) or func(identifier->member) or func(identifier[index]) +RESULTS=$(grep -rn --include="*.c" -P "($FORMAT_FUNCS)\s*\(\s*[a-zA-Z_][a-zA-Z0-9_]*(\s*->\s*\w+|\s*\[\s*\w+\s*\])?\s*\)\s*;" "$DIR" 2>/dev/null || true) + +# Filter out function definitions, declarations, and safe api_* wrappers +RESULTS=$(echo "$RESULTS" | grep -v "const char\|void \|^[^:]*:[0-9]*:[a-z_]*(\|api_cons_show\|api_log_" || true) + +if [ -n "$RESULTS" ]; then + echo "❌ POTENTIAL CWE-134 VULNERABILITIES FOUND:" + echo "" + echo "$RESULTS" + echo "" + ERRORS=$(echo "$RESULTS" | wc -l) +else + echo "✅ No obvious CWE-134 issues found." +fi + +# Additional check: GString->str passed directly (not as %s argument) +echo "" +echo "Checking for GString->str passed to format functions..." + +GSTRING_RESULTS=$(grep -rn --include="*.c" -P "($FORMAT_FUNCS)\s*\([^)]*->str\s*\)" "$DIR" 2>/dev/null | grep -v '"%s"' || true) + +if [ -n "$GSTRING_RESULTS" ]; then + echo "⚠️ GString->str passed without \"%s\" (review manually):" + echo "" + echo "$GSTRING_RESULTS" + echo "" +fi + +echo "" +echo "=== Summary ===" +echo "Critical issues: $ERRORS" + +if [ "$ERRORS" -gt 0 ]; then + echo "" + echo "Fix by adding \"%s\" format specifier:" + echo " BAD: cons_show(variable);" + echo " GOOD: cons_show(\"%s\", variable);" + exit 1 +fi + +exit 0 diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 98d01f31..f12c9038 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -4970,8 +4970,8 @@ cmd_sendfile(ProfWin* window, const char* const command, gchar** args) alt_scheme = OMEMO_AESGCM_URL_SCHEME; alt_fragment = _add_omemo_stream(&fd, &fh, &err); if (err != NULL) { - cons_show_error(err); - win_println(window, THEME_ERROR, "-", err); + cons_show_error("%s", err); + win_println(window, THEME_ERROR, "-", "%s", err); goto out; } #endif diff --git a/src/common.c b/src/common.c index 064eb757..69f90900 100644 --- a/src/common.c +++ b/src/common.c @@ -143,7 +143,7 @@ auto_close_gfd(gint* fd) return; if (close(*fd) == EOF) - log_error(g_strerror(errno)); + log_error("%s", g_strerror(errno)); } /** @@ -158,7 +158,7 @@ auto_close_FILE(FILE** fd) return; if (fclose(*fd) == EOF) - log_error(g_strerror(errno)); + log_error("%s", g_strerror(errno)); } static gboolean diff --git a/src/plugins/python_plugins.c b/src/plugins/python_plugins.c index 3b50c961..0e178501 100644 --- a/src/plugins/python_plugins.c +++ b/src/plugins/python_plugins.c @@ -887,8 +887,8 @@ _python_undefined_error(ProfPlugin* plugin, char* hook, char* type) g_string_append(err_msg, hook); g_string_append(err_msg, "(): return value undefined, expected "); g_string_append(err_msg, type); - log_error(err_msg->str); - cons_show_error(err_msg->str); + log_error("%s", err_msg->str); + cons_show_error("%s", err_msg->str); g_string_free(err_msg, TRUE); } @@ -901,8 +901,8 @@ _python_type_error(ProfPlugin* plugin, char* hook, char* type) g_string_append(err_msg, hook); g_string_append(err_msg, "(): incorrect return type, expected "); g_string_append(err_msg, type); - log_error(err_msg->str); - cons_show_error(err_msg->str); + log_error("%s", err_msg->str); + cons_show_error("%s", err_msg->str); g_string_free(err_msg, TRUE); } diff --git a/src/profanity.c b/src/profanity.c index 750f7891..b9b0cc87 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -135,7 +135,7 @@ prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_f */ min_runtime += waittime; } else { - log_error(err_msg); + log_error("%s", err_msg); g_free(err_msg); commands = NULL; } @@ -245,7 +245,7 @@ _init(char* log_level, char* config_file, char* log_file, char* theme_name) if (prof_log_level == PROF_LEVEL_DEBUG) { ProfWin* console = wins_get_console(); win_println(console, THEME_DEFAULT, "-", "Debug mode enabled! Logging to: "); - win_println(console, THEME_DEFAULT, "-", get_log_file_location()); + win_println(console, THEME_DEFAULT, "-", "%s", get_log_file_location()); } session_init(); cmd_init(); diff --git a/src/tools/http_upload.c b/src/tools/http_upload.c index a1c31b27..166797f1 100644 --- a/src/tools/http_upload.c +++ b/src/tools/http_upload.c @@ -311,7 +311,7 @@ http_file_put(void* userdata) } win_update_entry_message(upload->window, upload->put_url, err_msg); } - cons_show_error(err_msg); + cons_show_error("%s", err_msg); } else { if (!upload->cancel) { auto_gchar gchar* status_msg = g_strdup_printf("Uploading '%s': 100%%", upload->filename); @@ -327,7 +327,7 @@ http_file_put(void* userdata) if (!fail_msg) { fail_msg = g_strdup(FALLBACK_MSG); } - cons_show_error(fail_msg); + cons_show_error("%s", fail_msg); } else { switch (upload->window->type) { case WIN_CHAT: diff --git a/src/ui/console.c b/src/ui/console.c index 52c281e4..d1104fa0 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -938,7 +938,7 @@ cons_show_account_list(gchar** accounts) theme_item_t presence_colour = theme_main_presence_attrs(string_from_resource_presence(presence)); win_println(console, presence_colour, "-", "%s", accounts[i]); } else { - cons_show(accounts[i]); + cons_show("%s", accounts[i]); } } cons_show(""); diff --git a/src/ui/core.c b/src/ui/core.c index e61831a0..be77756c 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -445,7 +445,7 @@ ui_handle_error(const char* const err_msg) GString* msg = g_string_new(""); g_string_printf(msg, "Error %s", err_msg); - cons_show_error(msg->str); + cons_show_error("%s", msg->str); g_string_free(msg, TRUE); } diff --git a/src/ui/window.c b/src/ui/window.c index c4b8c92f..a652426c 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -2297,7 +2297,7 @@ void win_handle_command_exec_result_note(ProfWin* window, const char* const type, const char* const value) { assert(window != NULL); - win_println(window, THEME_DEFAULT, "!", value); + win_println(window, THEME_DEFAULT, "!", "%s", value); } void diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 30fb2696..9af7b8c4 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -868,7 +868,7 @@ _handle_error(xmpp_stanza_t* const stanza) g_string_append(log_msg, " error="); g_string_append(log_msg, err_msg); - log_info(log_msg->str); + log_info("%s", log_msg->str); g_string_free(log_msg, TRUE); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 05123d08..8916235a 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -455,7 +455,7 @@ _presence_error_handler(xmpp_stanza_t* const stanza) g_string_append(log_msg, " error="); g_string_append(log_msg, err_msg); - log_info(log_msg->str); + log_info("%s", log_msg->str); g_string_free(log_msg, TRUE); -- 2.49.1 From 56dcb54ffce1419a4388f415f89c238488cb3d67 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Fri, 6 Feb 2026 13:29:36 +0300 Subject: [PATCH 7/7] fix(ui): address PR review comments - inputwin.c: improve MB_CUR_MAX error message with actionable advice - inputwin.c: add log_error when input window pad allocation fails - statusbar.c: add log_warning for invalid cols values - window.c: simplify _check_subwin_width using CLAMP macro --- src/database.c | 2 ++ src/ui/inputwin.c | 5 +++-- src/ui/statusbar.c | 4 ++++ src/ui/window.c | 10 +--------- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/database.c b/src/database.c index 14837d00..4efd4873 100644 --- a/src/database.c +++ b/src/database.c @@ -77,6 +77,8 @@ _db_teardown(const char* ctx) } g_chatlog_database = NULL; } + // Safe to call unconditionally; no-op if not initialized. + // See: https://www.sqlite.org/c3ref/initialize.html sqlite3_shutdown(); } diff --git a/src/ui/inputwin.c b/src/ui/inputwin.c index 29a63130..4d25f832 100644 --- a/src/ui/inputwin.c +++ b/src/ui/inputwin.c @@ -148,7 +148,8 @@ create_input_window(void) * Fail gracefully instead of aborting in production. */ if (MB_CUR_MAX > PROF_MB_CUR_MAX) { - cons_show_error("Your locale's MB_CUR_MAX (%zu) exceeds PROF_MB_CUR_MAX (%d); input window disabled.", (size_t)MB_CUR_MAX, PROF_MB_CUR_MAX); + log_error("Locale MB_CUR_MAX (%zu) exceeds compiled limit (%d)", (size_t)MB_CUR_MAX, PROF_MB_CUR_MAX); + cons_show_error("Unsupported locale. Before running, execute in terminal: export LC_ALL=C.UTF-8"); return; } #ifdef NCURSES_REENTRANT @@ -167,7 +168,7 @@ create_input_window(void) inp_win = newpad(1, INP_WIN_MAX); if (!inp_win) { - // Failed to allocate input pad; leave inp_win NULL and avoid further use + log_error("Failed to allocate input window pad"); return; } wbkgd(inp_win, theme_attrs(THEME_INPUT_TEXT)); diff --git a/src/ui/statusbar.c b/src/ui/statusbar.c index ff74881a..49b37e8b 100644 --- a/src/ui/statusbar.c +++ b/src/ui/statusbar.c @@ -40,6 +40,8 @@ #include #include +#include "log.h" + #ifdef HAVE_NCURSESW_NCURSES_H #include #elif HAVE_NCURSES_H @@ -112,6 +114,7 @@ status_bar_init(void) int row = screen_statusbar_row(); int cols = getmaxx(stdscr); if (cols <= 0) { + log_warning("status_bar_init: invalid cols %d, defaulting to 1", cols); cols = 1; } statusbar_win = newwin(1, cols, row, 0); @@ -157,6 +160,7 @@ status_bar_resize(void) } int cols = getmaxx(stdscr); if (cols <= 0) { + log_warning("status_bar_resize: invalid cols %d, defaulting to 1", cols); cols = 1; } werase(statusbar_win); diff --git a/src/ui/window.c b/src/ui/window.c index a652426c..33fcac33 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -79,15 +79,7 @@ static void _win_print_wrapped(WINDOW* win, const char* const message, size_t in static int _check_subwin_width(int cols, int width) { - if (cols > 1) { - if (width < 1) - width = 1; - if (width >= cols) - width = cols - 1; - } else { - width = 1; - } - return width; + return cols <= 1 ? 1 : CLAMP(width, 1, cols - 1); } int -- 2.49.1