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
2026-06-14 17:06:37 +03:00
parent 7404e7092a
commit 7eb9668068
10 changed files with 86 additions and 31 deletions
--- 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\\\"\""
--- 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);
--- 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();

--- 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);

--- 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;
--- 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;
--- 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);
--- 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);
--- 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;
--- 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));
        }
    }