From 62361676feac228906c0e0f914e84b59742981c2 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Wed, 27 May 2026 14:28:20 +0300 Subject: [PATCH 1/6] fix: address issue #112 follow-ups (OTR strip, presence UAF, OMEMO load) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - otr: strip the OTR whitespace tag by shifting the full remaining tail (including the trailing NUL) instead of tag_length bytes, which duplicated the body for messages longer than the tag (#8). - xmpp: snapshot resource name/status/priority/presence before connection_add_available_resource() transfers ownership of the Resource, so the later reads can no longer become a use-after-free (#9). - omemo: propagate _omemo_finalize_identity_load failure on connect — mark the context unloaded, log it, surface a cons_show_error, and stop instead of silently leaving OMEMO unavailable (#10). Refs #112 --- src/omemo/omemo.c | 7 ++++++- src/otr/otr.c | 5 ++++- src/xmpp/presence.c | 22 +++++++++++++++------- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index 83911551..f31acb1d 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -260,7 +260,12 @@ omemo_on_connect(ProfAccount* account) return; } - _omemo_finalize_identity_load(account); + if (!_omemo_finalize_identity_load(account)) { + omemo_ctx.loaded = FALSE; + log_error("[OMEMO] failed to load OMEMO state from disk for %s", account->jid); + cons_show_error("OMEMO: could not load encryption state from disk; OMEMO will be unavailable this session."); + return; + } wins_omemo_trust_changed(NULL); } diff --git a/src/otr/otr.c b/src/otr/otr.c index 2c295225..ad382148 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -278,7 +278,10 @@ otr_on_message_recv(const char* const barejid, const char* const resource, const if (strstr(message, OTRL_MESSAGE_TAG_V2) && strstr(message, OTRL_MESSAGE_TAG_V1)) { tag_length = 32; } - memmove(whitespace_base, whitespace_base + tag_length, tag_length); + // Shift the remainder of the message (including the trailing NUL) + // left over the tag, not just the next tag_length bytes — otherwise + // the tail is left in place and the body is duplicated/truncated. + memmove(whitespace_base, whitespace_base + tag_length, strlen(whitespace_base + tag_length) + 1); char* otr_query_message = otr_start_query(); cons_show("OTR Whitespace pattern detected. Attempting to start OTR session…"); free(message_send_chat_otr(barejid, otr_query_message, FALSE, NULL)); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 27c7e7b0..c1a47765 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -621,15 +621,23 @@ _available_handler(xmpp_stanza_t* const stanza) } if (g_strcmp0(xmpp_presence->jid->barejid, my_jid->barejid) == 0) { - connection_add_available_resource(resource); + // Snapshot everything we read from `resource` before ownership is + // transferred to the available-resources table; dereferencing it + // afterwards would be a use-after-free should the same resource name + // ever be re-added between the transfer and the reads below. Resource* resource_for_roster = resource_copy(resource); + auto_gchar gchar* resource_name = g_strdup(resource->name); + auto_gchar gchar* resource_status = resource->status ? g_strdup(resource->status) : NULL; + int resource_priority = resource->priority; + resource_presence_t resource_presence = resource->presence; + connection_add_available_resource(resource); sv_ev_contact_online(xmpp_presence->jid->barejid, resource_for_roster, xmpp_presence->last_activity, pgpsig); const char* account_name = session_get_account_name(); int max_sessions = accounts_get_max_sessions(account_name); if (max_sessions > 0) { auto_gchar gchar* cur_resource = accounts_get_resource(account_name); int res_count = connection_count_available_resources(); - if (res_count > max_sessions && g_strcmp0(cur_resource, resource->name)) { + if (res_count > max_sessions && g_strcmp0(cur_resource, resource_name)) { ProfWin* console = wins_get_console(); ProfWin* current_window = wins_get_current(); auto_gchar gchar* message = g_strdup_printf("Max sessions alarm! (%d/%d devices in use)", res_count, max_sessions); @@ -639,14 +647,14 @@ _available_handler(xmpp_stanza_t* const stanza) } notify(message, 10000, "Security alert"); - const char* resource_presence = string_from_resource_presence(resource->presence); - win_print(console, THEME_DEFAULT, "|", "New device info: \n %s (%d), %s", resource->name, resource->priority, resource_presence); + const char* resource_presence_str = string_from_resource_presence(resource_presence); + win_print(console, THEME_DEFAULT, "|", "New device info: \n %s (%d), %s", resource_name, resource_priority, resource_presence_str); - if (resource->status) { - win_append(console, THEME_DEFAULT, ", \"%s\"", resource->status); + if (resource_status) { + win_append(console, THEME_DEFAULT, ", \"%s\"", resource_status); } win_appendln(console, THEME_DEFAULT, ""); - auto_jid Jid* jidp = jid_create_from_bare_and_resource(my_jid->barejid, resource->name); + auto_jid Jid* jidp = jid_create_from_bare_and_resource(my_jid->barejid, resource_name); EntityCapabilities* caps = caps_lookup(jidp->fulljid); if (caps) { -- 2.49.1 From 7404e7092a3abf83cdc274e6a2e5c8afa7339e21 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Fri, 5 Jun 2026 14:49:09 +0300 Subject: [PATCH 2/6] fix: more issue #112 follow-ups (stanza-id disco gate, account sanitizer) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - xmpp: gate XEP-0359 stanza-id dedup on disco urn:xmpp:sid:0. Before honoring for archive_id dedup, check that the entity in `by` (or its domain — that's what profanity disco's on connect) announces the namespace; fall back to no-dedup when caps are unknown, matching the XEP §6 disco-gate (#1). - accounts: narrow group-name sanitizer to only the characters GKeyFile actually forbids in group headers (`[`, `]`, `\n`, `\r`); `=` and `#` are valid inside a header and stay (#2). - functional tests cover both branches of the disco gate — replayed stanza-id is flagged as duplicate when sid:0 is announced, and not flagged when it isn't. Refs #112 --- src/config/accounts.c | 4 +- src/xmpp/message.c | 24 ++++++++- tests/functionaltests/functionaltests.c | 4 ++ tests/functionaltests/test_message.c | 67 +++++++++++++++++++++++++ tests/functionaltests/test_message.h | 2 + 5 files changed, 97 insertions(+), 4 deletions(-) diff --git a/src/config/accounts.c b/src/config/accounts.c index af6b901d..af5d34e4 100644 --- a/src/config/accounts.c +++ b/src/config/accounts.c @@ -43,8 +43,8 @@ _sanitize_account_name(const char* const name) gchar* sanitized = g_strdup(name); gchar* p = sanitized; while (*p) { - // GKeyFile special characters: [ ] = # \n \r - if (*p == '[' || *p == ']' || *p == '=' || *p == '#' || *p == '\n' || *p == '\r') { + // GKeyFile group header forbids these characters. + if (*p == '[' || *p == ']' || *p == '\n' || *p == '\r') { *p = '_'; } p++; diff --git a/src/xmpp/message.c b/src/xmpp/message.c index a7b2b058..c6040bf1 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -65,6 +65,7 @@ static void _handle_pubsub(xmpp_stanza_t* const stanza, xmpp_stanza_t* const eve static gboolean _handle_form(xmpp_stanza_t* const stanza); static gboolean _handle_jingle_message(xmpp_stanza_t* const stanza); static gboolean _should_ignore_based_on_silence(xmpp_stanza_t* const stanza); +static gboolean _stanza_id_by_trusted(const char* by); #ifdef HAVE_OMEMO static void _receive_omemo(xmpp_stanza_t* const stanza, ProfMessage* message); #endif @@ -1088,7 +1089,7 @@ _handle_groupchat(xmpp_stanza_t* const stanza) xmpp_stanza_t* stanzaidst = xmpp_stanza_get_child_by_name_and_ns(stanza, STANZA_NAME_STANZA_ID, STANZA_NS_STABLE_ID); if (stanzaidst) { const char* by = xmpp_stanza_get_attribute(stanzaidst, "by"); - if (by && (g_strcmp0(by, from_jid->barejid) == 0)) { + if (by && g_strcmp0(by, from_jid->barejid) == 0 && _stanza_id_by_trusted(by)) { stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); if (stanzaid) { message->stanzaid = strdup(stanzaid); @@ -1397,7 +1398,7 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c xmpp_stanza_t* stanzaidst = xmpp_stanza_get_child_by_name_and_ns(stanza, STANZA_NAME_STANZA_ID, STANZA_NS_STABLE_ID); if (stanzaidst) { const char* by = xmpp_stanza_get_attribute(stanzaidst, "by"); - if (by && equals_our_barejid(by)) { + if (by && equals_our_barejid(by) && _stanza_id_by_trusted(by)) { stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); if (stanzaid) { message->stanzaid = strdup(stanzaid); @@ -1735,3 +1736,22 @@ _should_ignore_based_on_silence(xmpp_stanza_t* const stanza) } return FALSE; } + +// XEP-0359 §6 disco-gate; falls back to domain since that's what disco is run against. +static gboolean +_stanza_id_by_trusted(const char* by) +{ + if (!by) { + return FALSE; + } + GHashTable* features = connection_get_features(by); + if (features && g_hash_table_contains(features, STANZA_NS_STABLE_ID)) { + return TRUE; + } + const char* at = strchr(by, '@'); + if (!at) { + return FALSE; + } + features = connection_get_features(at + 1); + return features && g_hash_table_contains(features, STANZA_NS_STABLE_ID); +} diff --git a/tests/functionaltests/functionaltests.c b/tests/functionaltests/functionaltests.c index 0ea473c3..8102f928 100644 --- a/tests/functionaltests/functionaltests.c +++ b/tests/functionaltests/functionaltests.c @@ -253,6 +253,10 @@ main(int argc, char* argv[]) PROF_FUNC_TEST(message_receive_console), PROF_FUNC_TEST(message_receive_chatwin), + /* XEP-0359 disco gate for stanza-id trust */ + PROF_FUNC_TEST(stanza_id_dedup_fires_when_server_announces_sid0), + PROF_FUNC_TEST(stanza_id_not_trusted_when_server_does_not_announce_sid0), + #ifdef HAVE_SQLITE /* MUC (groupchat) database — export/import of type="muc" messages */ PROF_FUNC_TEST(muc_export_sqlite_to_flatfile), diff --git a/tests/functionaltests/test_message.c b/tests/functionaltests/test_message.c index 74f5e6c3..402144ac 100644 --- a/tests/functionaltests/test_message.c +++ b/tests/functionaltests/test_message.c @@ -56,3 +56,70 @@ message_receive_chatwin(void **state) assert_true(prof_output_regex("someuser@chatserv.org/laptop: .+How are you?")); } + +// XEP-0359 disco gate: server announces urn:xmpp:sid:0 -> stanza-id trusted -> replay flagged as duplicate. +void +stanza_id_dedup_fires_when_server_announces_sid0(void **state) +{ + stbbr_for_query("http://jabber.org/protocol/disco#info", + "" + "" + "" + "" + "" + "" + ); + + prof_connect(); + + stbbr_send( + "" + "first" + "" + "" + ); + assert_true(prof_output_exact("<< chat message: someuser@chatserv.org/laptop (win 2)")); + + stbbr_send( + "" + "replay" + "" + "" + ); + assert_true(prof_output_exact("Got a message with duplicate (server-generated) stanza-id from someuser@chatserv.org/laptop.")); +} + +// XEP-0359 disco gate: server does NOT announce urn:xmpp:sid:0 -> stanza-id untrusted -> no replay error. +void +stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state) +{ + stbbr_for_query("http://jabber.org/protocol/disco#info", + "" + "" + "" + "" + "" + "" + ); + + prof_connect(); + + stbbr_send( + "" + "first" + "" + "" + ); + assert_true(prof_output_exact("<< chat message: someuser@chatserv.org/laptop (win 2)")); + + stbbr_send( + "" + "replay" + "" + "" + ); + + prof_timeout(2); + assert_false(prof_output_exact("Got a message with duplicate (server-generated) stanza-id")); + prof_timeout_reset(); +} diff --git a/tests/functionaltests/test_message.h b/tests/functionaltests/test_message.h index d3a1987c..924ad914 100644 --- a/tests/functionaltests/test_message.h +++ b/tests/functionaltests/test_message.h @@ -1,3 +1,5 @@ void message_send(void **state); void message_receive_console(void **state); void message_receive_chatwin(void **state); +void stanza_id_dedup_fires_when_server_announces_sid0(void **state); +void stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state); -- 2.49.1 From 7eb966806869efb6650d4d7e77af9ad7ce66c4d2 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Sun, 14 Jun 2026 17:06:37 +0300 Subject: [PATCH 3/6] build: Pikaur-safe hardening flags, opt-in sanitizers, -Wsign-compare Re-introduce the build hardening that 5459e78e8 attempted, in a form that coexists with Arch/Pikaur builds (the conflict that forced its revert in 0722dc9e3). configure.ac: - Add -D_FORTIFY_SOURCE=2 only when neither $CFLAGS nor $CPPFLAGS already define _FORTIFY_SOURCE (makepkg injects its own) and not under --coverage. - Route glib/gio CFLAGS through -isystem so their macros don't trip our -W set. - Add --enable-sanitizers (ASan + UBSan; unsigned-integer-overflow probed, skipped on gcc) for a dedicated CI job, off by default. - Add --enable-hardening to gate -Wnull-dereference (false positives under -O2 on some toolchains), off by default. - Promote -Wsign-compare from -Wno- to an active probed warning. Also fix the sign-conversion sites that -Wsign-compare/-Wconversion cleanup surfaced as safe (pointer-diff to gsize via new g_diff_to_gsize helper, non-negative int casts), and validate the /vcard remove index through strtoi_range so a negative argument can no longer become a huge unsigned index. Refs #112 --- configure.ac | 44 ++++++++++++++++++++++++++++++++-- src/command/cmd_funcs.c | 10 ++++++-- src/common.c | 15 ++++++------ src/common.h | 8 +++++++ src/database_export.c | 4 ++-- src/database_flatfile.c | 12 +++++----- src/database_flatfile_parser.c | 16 ++++++------- src/profanity.c | 2 +- src/xmpp/jid.c | 4 ++-- src/xmpp/muc.c | 2 +- 10 files changed, 86 insertions(+), 31 deletions(-) diff --git a/configure.ac b/configure.ac index 37034826..dece25a2 100644 --- a/configure.ac +++ b/configure.ac @@ -71,6 +71,10 @@ AC_ARG_ENABLE([omemo-qrcode], [AS_HELP_STRING([--enable-omemo-qrcode], [enable ability to display omemo qr code])]) AC_ARG_ENABLE([coverage], [AS_HELP_STRING([--enable-coverage], [enable code coverage analysis])]) +AC_ARG_ENABLE([sanitizers], + [AS_HELP_STRING([--enable-sanitizers], [enable AddressSanitizer + UndefinedBehaviorSanitizer + unsigned-integer-overflow])]) +AC_ARG_ENABLE([hardening], + [AS_HELP_STRING([--enable-hardening], [enable extra hardening warnings (e.g. -Wnull-dereference) that may produce false positives under -O2])]) m4_include([m4/ax_valgrind_check.m4]) AX_VALGRIND_DFLT([drd], [off]) @@ -395,7 +399,7 @@ AC_SUBST([FORKPTY_LIB]) ## Default parameters AM_CFLAGS="$AM_CFLAGS -Wall -Wextra -Wformat=2 -Wno-format-zero-length" -AM_CFLAGS="$AM_CFLAGS -Wno-deprecated-declarations -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare -Wno-cast-function-type" +AM_CFLAGS="$AM_CFLAGS -Wno-deprecated-declarations -Wno-unused-parameter -Wno-missing-field-initializers -Wno-cast-function-type" AM_CFLAGS="$AM_CFLAGS -Wpointer-arith" AM_CFLAGS="$AM_CFLAGS -Wimplicit-function-declaration" AM_CFLAGS="$AM_CFLAGS -Wundef" @@ -406,7 +410,8 @@ AM_CFLAGS="$AM_CFLAGS -std=gnu99 -ggdb3" # GCC-specific warnings (not supported by clang) — test each one saved_CFLAGS="$CFLAGS" for _flag in -Wlogical-op -Wduplicated-cond -Wduplicated-branches \ - -Wstringop-overflow -Warray-bounds=2 -Walloc-zero; do + -Wstringop-overflow -Warray-bounds=2 -Walloc-zero \ + -Wsign-compare; do AC_MSG_CHECKING([whether $CC supports $_flag]) CFLAGS="$saved_CFLAGS $_flag -Werror" AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], @@ -433,11 +438,46 @@ AS_IF([test "x$enable_coverage" = xyes], AM_LDFLAGS="$AM_LDFLAGS --coverage" AC_MSG_NOTICE([Code coverage analysis enabled])]) +AS_IF([test "x$enable_sanitizers" = xyes], + [SAN_FLAGS="-fsanitize=address,undefined" + saved_CFLAGS="$CFLAGS" + CFLAGS="$saved_CFLAGS -fsanitize=unsigned-integer-overflow -Werror" + AC_MSG_CHECKING([whether $CC supports -fsanitize=unsigned-integer-overflow]) + AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], + [AC_MSG_RESULT([yes]); SAN_FLAGS="$SAN_FLAGS,unsigned-integer-overflow"], + [AC_MSG_RESULT([no])]) + CFLAGS="$saved_CFLAGS" + AM_CFLAGS="$AM_CFLAGS $SAN_FLAGS -fno-sanitize-recover=all -fno-omit-frame-pointer" + AM_LDFLAGS="$AM_LDFLAGS -fsanitize=address,undefined" + AC_MSG_NOTICE([Sanitizers enabled: $SAN_FLAGS])]) + +AS_IF([test "x$enable_hardening" = xyes], + [saved_CFLAGS="$CFLAGS" + for _flag in -Wnull-dereference; do + AC_MSG_CHECKING([whether $CC supports $_flag]) + CFLAGS="$saved_CFLAGS $_flag -Werror" + AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], + [AC_MSG_RESULT([yes]); AM_CFLAGS="$AM_CFLAGS $_flag"], + [AC_MSG_RESULT([no])]) + done + CFLAGS="$saved_CFLAGS" + AC_MSG_NOTICE([Extra hardening warnings enabled])]) + +# Skip _FORTIFY_SOURCE=2 if env already set one (Pikaur/makepkg) or under coverage -O0 (FORTIFY needs -O>0). +AS_IF([test "x$enable_coverage" != xyes], + [AS_CASE([" ${CPPFLAGS} ${CFLAGS} "], + [*_FORTIFY_SOURCE=*], [AC_MSG_NOTICE([_FORTIFY_SOURCE preset from environment, leaving as-is])], + [AM_CFLAGS="$AM_CFLAGS -D_FORTIFY_SOURCE=2"])]) + AS_IF([test "x$PACKAGE_STATUS" = xdevelopment], [AM_CFLAGS="$AM_CFLAGS -Wunused -Werror"]) AS_IF([test "x$PLATFORM" = xosx], [AM_CFLAGS="$AM_CFLAGS -Qunused-arguments"]) +# Treat glib/gio headers as system to keep their macros out of our -W* set. +glib_CFLAGS=$(echo "$glib_CFLAGS" | sed 's/-I/-isystem /g') +gio_CFLAGS=$(echo "$gio_CFLAGS" | sed 's/-I/-isystem /g') + AM_CFLAGS="$AM_CFLAGS $PTHREAD_CFLAGS $glib_CFLAGS $gio_CFLAGS $curl_CFLAGS ${SQLITE_CFLAGS}" AM_CFLAGS="$AM_CFLAGS $libnotify_CFLAGS ${GTK_CFLAGS} $python_CFLAGS" AM_CFLAGS="$AM_CFLAGS -DTHEMES_PATH=\"\\\"$THEMES_PATH\\\"\" -DICONS_PATH=\"\\\"$ICONS_PATH\\\"\" -DGLOBAL_PYTHON_PLUGINS_PATH=\"\\\"$GLOBAL_PYTHON_PLUGINS_PATH\\\"\" -DGLOBAL_C_PLUGINS_PATH=\"\\\"$GLOBAL_C_PLUGINS_PATH\\\"\"" diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index a5ff1eec..827212f1 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -10063,8 +10063,14 @@ cmd_vcard_remove(ProfWin* window, const char* const command, gchar** args) } if (args[1]) { - vcard_user_remove_element(atoi(args[1])); - cons_show("Removed element at index %d", atoi(args[1])); + int index; + auto_gchar gchar* err_msg = NULL; + if (!strtoi_range(args[1], &index, 0, INT_MAX, &err_msg)) { + cons_show("%s", err_msg); + return TRUE; + } + vcard_user_remove_element((unsigned int)index); + cons_show("Removed element at index %d", index); vcardwin_update(); } else { cons_bad_cmd_usage(command); diff --git a/src/common.c b/src/common.c index 6a0e24fa..bc70b2a3 100644 --- a/src/common.c +++ b/src/common.c @@ -294,7 +294,7 @@ str_replace(const char* string, const char* substr, wr = newstr; while ((tok = strstr(head, substr))) { - size_t l = tok - head; + size_t l = g_diff_to_gsize(tok, head); memcpy(wr, head, l); wr += l; memcpy(wr, replacement, len_replacement); @@ -359,9 +359,9 @@ string_matches_one_of(const char* what, const char* is, gboolean is_can_be_null, char errmsg[256] = { 0 }; size_t sz = 0; int s = snprintf(errmsg, sizeof(errmsg) - sz, "%s must be one of:", what); - if (s < 0 || s + sz >= sizeof(errmsg)) + if (s < 0 || (size_t)s + sz >= sizeof(errmsg)) return ret; - sz += s; + sz += (size_t)s; cur = first; va_start(ap, first); @@ -375,12 +375,12 @@ string_matches_one_of(const char* what, const char* is, gboolean is_can_be_null, errmsg[sz] = '\0'; s = snprintf(errmsg + sz, sizeof(errmsg) - sz, " or '%s'.", cur); } - if (s < 0 || s + sz >= sizeof(errmsg)) { + if (s < 0 || (size_t)s + sz >= sizeof(errmsg)) { log_debug("Error message too long or some other error occurred (%d).", s); s = -1; break; } - sz += s; + sz += (size_t)s; cur = next; } va_end(ap); @@ -431,7 +431,7 @@ str_xml_sanitize(const char* const str) return NULL; } - GString* sanitized = g_string_new_len(NULL, strlen(str)); + GString* sanitized = g_string_new_len(NULL, (gssize)strlen(str)); const char* curr = str; while (*curr != '\0') { @@ -692,12 +692,13 @@ get_file_paths_recursive(const char* path, GSList** contents) gchar* get_random_string(int length) { + g_assert(length >= 0); GRand* prng; gchar* rand; gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; int endrange = sizeof(alphabet) - 1; - rand = g_malloc0(length + 1); + rand = g_malloc0((gsize)length + 1); prng = g_rand_new(); diff --git a/src/common.h b/src/common.h index 40c68d60..efe89cf9 100644 --- a/src/common.h +++ b/src/common.h @@ -158,6 +158,14 @@ gboolean create_dir(const char* name); gboolean copy_file(const char* const src, const char* const target, const gboolean overwrite_existing); char* str_replace(const char* string, const char* substr, const char* replacement); gboolean strtoi_range(const char* str, int* saveptr, int min, int max, char** err_msg); + +// Pointer-diff to gsize for callers that know end >= start by construction. +static inline gsize +g_diff_to_gsize(const void* end, const void* start) +{ + g_assert(end >= start); + return (gsize)((const char*)end - (const char*)start); +} int utf8_display_len(const char* const str); gchar* str_xml_sanitize(const char* const str); diff --git a/src/database_export.c b/src/database_export.c index 7e21e368..d4954a1f 100644 --- a/src/database_export.c +++ b/src/database_export.c @@ -282,7 +282,7 @@ _export_one_contact(const char* contact) gchar* key = _make_dedup_key(pl->stanza_id, pl->timestamp_str, pl->from_jid, pl->message); g_hash_table_add(seen_keys, key); } - int existing_count = g_slist_length(existing); + int existing_count = (int)g_slist_length(existing); // 2. Query SQLite for this contact — get ALL messages via direct SQL GSList* sqlite_lines = db_sqlite_get_all_chat(contact); @@ -581,7 +581,7 @@ log_database_import_from_flatfile(const gchar* const contact_jid) // Wrap in a transaction for atomicity and performance int contact_imported = 0; int contact_skipped = 0; - int total_lines = g_slist_length(ff_lines); + int total_lines = (int)g_slist_length(ff_lines); db_sqlite_begin_transaction(); gboolean import_ok = TRUE; diff --git a/src/database_flatfile.c b/src/database_flatfile.c index 5c70eb08..8c4222fe 100644 --- a/src/database_flatfile.c +++ b/src/database_flatfile.c @@ -88,7 +88,7 @@ _ff_cache_line_ids(ff_contact_state_t* state, const char* line) return; // Split metadata on unescaped '|' - auto_gchar gchar* meta_str = g_strndup(bracket + 1, close - bracket - 1); + auto_gchar gchar* meta_str = g_strndup(bracket + 1, g_diff_to_gsize(close, bracket + 1)); char** parts = ff_split_meta(meta_str); char* stanza_id = NULL; @@ -111,9 +111,9 @@ _ff_cache_line_ids(ff_contact_state_t* state, const char* line) const char* sender_start = close + 2; const char* colonspace = ff_find_unescaped_colonspace(sender_start); if (colonspace) { - const char* slash = memchr(sender_start, '/', colonspace - sender_start); + const char* slash = memchr(sender_start, '/', g_diff_to_gsize(colonspace, sender_start)); const char* jid_end = slash ? slash : colonspace; - from_jid = g_strndup(sender_start, jid_end - sender_start); + from_jid = g_strndup(sender_start, g_diff_to_gsize(jid_end, sender_start)); } } @@ -140,7 +140,7 @@ _ff_maybe_index_line(ff_contact_state_t* state, const char* buf, off_t pos) if (!space) return; - auto_gchar gchar* ts_str = g_strndup(buf, space - buf); + auto_gchar gchar* ts_str = g_strndup(buf, g_diff_to_gsize(space, buf)); GDateTime* dt = g_date_time_new_from_iso8601(ts_str, NULL); if (!dt) { log_warning("flatfile: unparsable timestamp in %s at offset %ld: %s", @@ -771,7 +771,7 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta // For "last N messages": back up from read_to using index if (!from_start) { - int margin_entries = (MESSAGES_TO_RETRIEVE * 3) / FF_INDEX_STEP + 2; + size_t margin_entries = (MESSAGES_TO_RETRIEVE * 3) / FF_INDEX_STEP + 2; // Find index entry closest to (but before) read_to size_t entry_idx = 0; @@ -781,7 +781,7 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta entry_idx = i; } - size_t start_entry = entry_idx > (size_t)margin_entries + size_t start_entry = entry_idx > margin_entries ? entry_idx - margin_entries : 0; off_t backed = state->entries[start_entry].byte_offset; diff --git a/src/database_flatfile_parser.c b/src/database_flatfile_parser.c index 932878df..7e6120f2 100644 --- a/src/database_flatfile_parser.c +++ b/src/database_flatfile_parser.c @@ -553,7 +553,7 @@ ff_split_meta(const char* meta) continue; } if (*p == '|' || *p == '\0') { - g_ptr_array_add(arr, g_strndup(start, p - start)); + g_ptr_array_add(arr, g_strndup(start, g_diff_to_gsize(p, start))); if (*p == '\0') break; start = p + 1; @@ -710,7 +710,7 @@ ff_parse_line(const char* line) if (bracket_start && first_space && first_space < bracket_start) { // Standard format with metadata: {timestamp} [{meta}] {sender}: {msg} - result->timestamp_str = g_strndup(work, first_space - work); + result->timestamp_str = g_strndup(work, g_diff_to_gsize(first_space, work)); // Parse metadata section [...] const char* bracket_end = ff_find_unescaped_char(bracket_start + 1, ']'); @@ -721,7 +721,7 @@ ff_parse_line(const char* line) return NULL; } - auto_gchar gchar* meta_content = g_strndup(bracket_start + 1, bracket_end - bracket_start - 1); + auto_gchar gchar* meta_content = g_strndup(bracket_start + 1, g_diff_to_gsize(bracket_end, bracket_start + 1)); char** parts = ff_split_meta(meta_content); if (parts) { _ff_parse_meta_parts(parts, result); @@ -736,12 +736,12 @@ ff_parse_line(const char* line) // Find first *unescaped* ': ' which separates sender from message. const char* colon = ff_find_unescaped_colonspace(after_meta); if (colon) { - auto_gchar gchar* raw_sender = g_strndup(after_meta, colon - after_meta); + auto_gchar gchar* raw_sender = g_strndup(after_meta, g_diff_to_gsize(colon, after_meta)); // Split sender into jid/resource, then unescape resource char* slash = strchr(raw_sender, '/'); if (slash) { - result->from_jid = g_strndup(raw_sender, slash - raw_sender); + result->from_jid = g_strndup(raw_sender, g_diff_to_gsize(slash, raw_sender)); result->from_resource = ff_unescape_sender_resource(slash + 1); } else { result->from_jid = g_strdup(raw_sender); @@ -755,7 +755,7 @@ ff_parse_line(const char* line) } } else if (first_space) { // Legacy/simple format without metadata: {timestamp} - {sender}: {msg} - result->timestamp_str = g_strndup(work, first_space - work); + result->timestamp_str = g_strndup(work, g_diff_to_gsize(first_space, work)); result->type = g_strdup("chat"); result->enc = g_strdup("none"); @@ -767,10 +767,10 @@ ff_parse_line(const char* line) char* colon = strstr(rest, ": "); if (colon) { - char* sender = g_strndup(rest, colon - rest); + char* sender = g_strndup(rest, g_diff_to_gsize(colon, rest)); char* slash = strchr(sender, '/'); if (slash) { - result->from_jid = g_strndup(sender, slash - sender); + result->from_jid = g_strndup(sender, g_diff_to_gsize(slash, sender)); result->from_resource = g_strdup(slash + 1); } else { result->from_jid = g_strdup(sender); diff --git a/src/profanity.c b/src/profanity.c index 08655295..4477d6a9 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -138,7 +138,7 @@ prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_f * so we can be sure there's runtime left after executing * the last command. */ - min_runtime += waittime; + min_runtime += (unsigned int)waittime; } else { log_error("%s", err_msg); g_free(err_msg); diff --git a/src/xmpp/jid.c b/src/xmpp/jid.c index c36538e1..1a191dcf 100644 --- a/src/xmpp/jid.c +++ b/src/xmpp/jid.c @@ -117,7 +117,7 @@ jid_is_valid(const gchar* const str) // Localpart validation if (at) { - size_t local_len = at - str; + size_t local_len = g_diff_to_gsize(at, str); if (local_len == 0 || local_len > JID_MAX_PART_LEN) { return FALSE; } @@ -135,7 +135,7 @@ jid_is_valid(const gchar* const str) // Resourcepart validation if present if (slash) { - domain_len = slash - domain_start; + domain_len = g_diff_to_gsize(slash, domain_start); size_t resource_len = strlen(slash + 1); if (resource_len > JID_MAX_PART_LEN) { return FALSE; diff --git a/src/xmpp/muc.c b/src/xmpp/muc.c index 472c7ee3..da9c925d 100644 --- a/src/xmpp/muc.c +++ b/src/xmpp/muc.c @@ -707,7 +707,7 @@ muc_autocomplete(ProfWin* window, const char* const input, gboolean previous) } else { search_str = last_space + 1; if (!chat_room->autocomplete_prefix) { - chat_room->autocomplete_prefix = g_strndup(input, search_str - input); + chat_room->autocomplete_prefix = g_strndup(input, g_diff_to_gsize(search_str, input)); } } -- 2.49.1 From a4e6419faf635258e0c68cc74a8e667fb799d886 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Sun, 14 Jun 2026 17:29:50 +0300 Subject: [PATCH 4/6] test: fix -Wsign-compare in proftest _mkdir_recursive loop The functional-test harness compared a signed loop index against strlen()'s size_t, which the newly promoted -Wsign-compare turns into a -Werror failure when the test binaries are built. Make the index size_t. Refs #112 --- tests/functionaltests/proftest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functionaltests/proftest.c b/tests/functionaltests/proftest.c index ec5dc621..d8cbe739 100644 --- a/tests/functionaltests/proftest.c +++ b/tests/functionaltests/proftest.c @@ -119,7 +119,7 @@ _create_dir(const char* name) gboolean _mkdir_recursive(const char* dir) { - int i; + size_t i; gboolean result = TRUE; for (i = 1; i <= strlen(dir); i++) { -- 2.49.1 From f16b26bd6d31b3d91f1f441ebf85d9d58584c04a Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Tue, 16 Jun 2026 18:03:50 +0300 Subject: [PATCH 5/6] fix(review): address PR #130 review - proftest: declare the _mkdir_recursive loop index inside the for. - xmpp/presence: shorten the resource-snapshot comment to one line. - command/cmd_funcs: report the /vcard remove index error via cons_show_error. - common: replace g_assert in g_diff_to_gsize with a log_error + return 0 (moved out of the header into common.c); take get_random_string length as size_t instead of asserting non-negative. - otr: drop the change-narrating comment on the whitespace memmove. Refs #112 --- src/command/cmd_funcs.c | 2 +- src/common.c | 17 +++++++++++++---- src/common.h | 11 ++--------- src/otr/otr.c | 4 +--- src/xmpp/presence.c | 5 +---- tests/functionaltests/proftest.c | 3 +-- 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 827212f1..3fad000a 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -10066,7 +10066,7 @@ cmd_vcard_remove(ProfWin* window, const char* const command, gchar** args) int index; auto_gchar gchar* err_msg = NULL; if (!strtoi_range(args[1], &index, 0, INT_MAX, &err_msg)) { - cons_show("%s", err_msg); + cons_show_error("%s", err_msg); return TRUE; } vcard_user_remove_element((unsigned int)index); diff --git a/src/common.c b/src/common.c index bc70b2a3..477c41b4 100644 --- a/src/common.c +++ b/src/common.c @@ -309,6 +309,16 @@ str_replace(const char* string, const char* substr, return newstr; } +gsize +g_diff_to_gsize(const void* end, const void* start) +{ + if (end < start) { + log_error("g_diff_to_gsize: end < start (%p < %p)", end, start); + return 0; + } + return (gsize)((const char*)end - (const char*)start); +} + gboolean strtoi_range(const char* str, int* saveptr, int min, int max, gchar** err_msg) { @@ -690,19 +700,18 @@ get_file_paths_recursive(const char* path, GSList** contents) } gchar* -get_random_string(int length) +get_random_string(size_t length) { - g_assert(length >= 0); GRand* prng; gchar* rand; gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; int endrange = sizeof(alphabet) - 1; - rand = g_malloc0((gsize)length + 1); + rand = g_malloc0(length + 1); prng = g_rand_new(); - for (int i = 0; i < length; i++) { + for (size_t i = 0; i < length; i++) { rand[i] = alphabet[g_rand_int_range(prng, 0, endrange)]; } g_rand_free(prng); diff --git a/src/common.h b/src/common.h index efe89cf9..7abcb9fd 100644 --- a/src/common.h +++ b/src/common.h @@ -158,14 +158,7 @@ gboolean create_dir(const char* name); gboolean copy_file(const char* const src, const char* const target, const gboolean overwrite_existing); char* str_replace(const char* string, const char* substr, const char* replacement); gboolean strtoi_range(const char* str, int* saveptr, int min, int max, char** err_msg); - -// Pointer-diff to gsize for callers that know end >= start by construction. -static inline gsize -g_diff_to_gsize(const void* end, const void* start) -{ - g_assert(end >= start); - return (gsize)((const char*)end - (const char*)start); -} +gsize g_diff_to_gsize(const void* end, const void* start); int utf8_display_len(const char* const str); gchar* str_xml_sanitize(const char* const str); @@ -186,7 +179,7 @@ int is_regular_file(const char* path); int is_dir(const char* path); void get_file_paths_recursive(const char* directory, GSList** contents); -gchar* get_random_string(int length); +gchar* get_random_string(size_t length); gboolean call_external(gchar** argv); gchar** format_call_external_argv(const char* template_fmt, const char* url, const char* filename); diff --git a/src/otr/otr.c b/src/otr/otr.c index ad382148..b5c336a9 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -278,9 +278,7 @@ otr_on_message_recv(const char* const barejid, const char* const resource, const if (strstr(message, OTRL_MESSAGE_TAG_V2) && strstr(message, OTRL_MESSAGE_TAG_V1)) { tag_length = 32; } - // Shift the remainder of the message (including the trailing NUL) - // left over the tag, not just the next tag_length bytes — otherwise - // the tail is left in place and the body is duplicated/truncated. + // Move the rest of the message (with NUL) over the tag. memmove(whitespace_base, whitespace_base + tag_length, strlen(whitespace_base + tag_length) + 1); char* otr_query_message = otr_start_query(); cons_show("OTR Whitespace pattern detected. Attempting to start OTR session…"); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index c1a47765..8ab4625d 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -621,10 +621,7 @@ _available_handler(xmpp_stanza_t* const stanza) } if (g_strcmp0(xmpp_presence->jid->barejid, my_jid->barejid) == 0) { - // Snapshot everything we read from `resource` before ownership is - // transferred to the available-resources table; dereferencing it - // afterwards would be a use-after-free should the same resource name - // ever be re-added between the transfer and the reads below. + // Copy what we read before connection_add_available_resource() takes ownership. Resource* resource_for_roster = resource_copy(resource); auto_gchar gchar* resource_name = g_strdup(resource->name); auto_gchar gchar* resource_status = resource->status ? g_strdup(resource->status) : NULL; diff --git a/tests/functionaltests/proftest.c b/tests/functionaltests/proftest.c index d8cbe739..4b202c26 100644 --- a/tests/functionaltests/proftest.c +++ b/tests/functionaltests/proftest.c @@ -119,10 +119,9 @@ _create_dir(const char* name) gboolean _mkdir_recursive(const char* dir) { - size_t i; gboolean result = TRUE; - for (i = 1; i <= strlen(dir); i++) { + for (size_t i = 1; i <= strlen(dir); i++) { if (dir[i] == '/' || dir[i] == '\0') { gchar* next_dir = g_strndup(dir, i); result = _create_dir(next_dir); -- 2.49.1 From f63bf578723919d04ed050290c9030c967aa08d4 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Fri, 19 Jun 2026 11:29:58 +0300 Subject: [PATCH 6/6] fix(omemo): guard NULL fingerprint decode and log failures (CWE-476/457) Absorbs the still-relevant null-pointer guards from #107 and extends them for consistency across all _omemo_fingerprint_decode() callers: - _omemo_fingerprint_decode(): return NULL on NULL input instead of dereferencing it via strlen(). - omemo_is_trusted_identity(): bail out (FALSE) when decode fails and zero-init fingerprint_len, instead of passing a NULL buffer / reading an uninitialized length into signal_buffer_append(). - omemo_trust(): bail out when decode fails (previously unchecked; under OOM it would save a bogus 1-byte identity). Every decode failure is now logged (omemo_is_trusted_identity / omemo_trust / omemo_untrust) instead of failing silently; omemo_trust also surfaces a user-facing error. The cmd_funcs.c hunks from #107 are dropped: master already has the strtok guard, and its fdopen guard is more complete (it closes the fd). --- src/omemo/omemo.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index f31acb1d..38157336 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -1283,8 +1283,12 @@ omemo_is_trusted_identity(const char* const jid, const char* const fingerprint) .device_id = GPOINTER_TO_UINT(device_id), }; - size_t fingerprint_len; + size_t fingerprint_len = 0; unsigned char* fingerprint_raw = _omemo_fingerprint_decode(fingerprint, &fingerprint_len); + if (!fingerprint_raw) { + log_error("[OMEMO] omemo_is_trusted_identity: failed to decode fingerprint for %s", jid); + return FALSE; + } unsigned char djb_type[] = { '\x05' }; signal_buffer* buffer = signal_buffer_create(djb_type, 1); buffer = signal_buffer_append(buffer, fingerprint_raw, fingerprint_len); @@ -1468,6 +1472,10 @@ _omemo_fingerprint(ec_public_key* identity, gboolean formatted) static unsigned char* _omemo_fingerprint_decode(const char* const fingerprint, size_t* len) { + if (!fingerprint) { + *len = 0; + return NULL; + } unsigned char* output = malloc(strlen(fingerprint) / 2 + 1); if (!output) { *len = 0; @@ -1534,6 +1542,11 @@ omemo_trust(const char* const jid, const char* const fingerprint_formatted) }; unsigned char* fingerprint_raw = _omemo_fingerprint_decode(fingerprint_formatted, &len); + if (!fingerprint_raw) { + log_error("[OMEMO] omemo_trust: failed to decode fingerprint for %s", jid); + cons_show_error("Failed to trust device: could not decode fingerprint."); + return; + } unsigned char djb_type[] = { '\x05' }; signal_buffer* buffer = signal_buffer_create(djb_type, 1); buffer = signal_buffer_append(buffer, fingerprint_raw, len); @@ -1551,8 +1564,10 @@ omemo_untrust(const char* const jid, const char* const fingerprint_formatted) { size_t len; unsigned char* identity = _omemo_fingerprint_decode(fingerprint_formatted, &len); - if (!identity) + if (!identity) { + log_error("[OMEMO] omemo_untrust: failed to decode fingerprint for %s", jid); return; + } GHashTableIter iter; gpointer key, value; -- 2.49.1