[#49] harden NULL handling and resource lifecycle across UI and SQLite #47
Reference in New Issue
Block a user
No description provided.
Delete Branch "playground/fix/src_refactoring"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Refactoring: security hardening, code deduplication, test coverage
Security
cons_show,log_error,win_printlncalls where user data was passed directly as format stringRefactoring
g_hash_table_iterinstead ofg_hash_table_foreach(#58)cmd_defs.ccommand definitions for improved metrics collectionTests
CI
Docs
Closes #49, #58, #85
fix(ui,db): harden NULL handling and resource lifecycle across UI and SQLiteto [#49] harden NULL handling and resource lifecycle across UI and SQLite[#49] harden NULL handling and resource lifecycle across UI and SQLiteto WIP: [#49] harden NULL handling and resource lifecycle across UI and SQLite- 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@@ -68,0 +77,4 @@}g_chatlog_database = NULL;}sqlite3_shutdown();Earlier it was conditional. Could it cause an issue to have unconditional sqlite3_shutdown?
Also the condition takes almost full function. Maybe it would make sense to use early exit statement
if (!g_chatlog_database) return;?Unconditional sqlite3_shutdown() is safe — it's a no-op if SQLite is already shut down.
https://www.sqlite.org/c3ref/initialize.html
@@ -286,0 +315,4 @@if (!myjid || !myjid->str) {// If caller requested the last message and we have no context, fall back to nowif (is_last) {msg->timestamp = g_date_time_new_now_utc();Wouldn't this cause a faulty logic on
iq_mam_request_older?@@ -302,1 +338,3 @@return NULL;if (!_db_prepare_ctx(query, &stmt, "log_database_get_limits_info()")) {if (is_last) {msg->timestamp = g_date_time_new_now_utc();Analogous to the previous comment.
@@ -164,2 +167,4 @@inp_win = newpad(1, INP_WIN_MAX);if (!inp_win) {// Failed to allocate input pad; leave inp_win NULL and avoid further useMaybe we should add error logging for gracefulness?
Not resolved; no response.
Added
@@ -111,16 +111,23 @@ status_bar_init(void)int row = screen_statusbar_row();int cols = getmaxx(stdscr);if (cols <= 0) {Is it realistic condition? It doesn't appear a normal state, so maybe we should log_warn it?
Not resolved, as no response given/action taken.
Added
@@ -676,3 +692,2 @@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) {Nice catch!
src/ui/console.c:941
b7a38f1e34to58dd89be4058c23c4089to7d7615f10d7d7615f10dto22335ca83e22335ca83eto34501d913cPlease fix/comment earlier suggestions and address these 2 moments.
@@ -149,2 +149,3 @@*/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);Technically correct, but what user is supposed to do with this data? Can we provide some actionable advise or explanation?
Comment added
@@ -78,0 +79,4 @@static int_check_subwin_width(int cols, int width){if (cols > 1) {Why not like this?
Corrected
WIP: [#49] harden NULL handling and resource lifecycle across UI and SQLiteto [#49] harden NULL handling and resource lifecycle across UI and SQLiteSuggested squash commit message:
fix(ui,db): harden NULL handling, fix CWE-134, optimize iterations
Closes #58, #85
7d392e38catocbeaa13ed0cbeaa13ed0to56dcb54ffcLGTM