fix: CWE-134 format string vulnerability and compiler flags audit #98

Manually merged
jabber.developer merged 4 commits from fix/cwe-134-format-string-audit into master 2026-03-07 10:57:24 +00:00
Collaborator

Summary

Compiler flags hardening, CWE-134 format string audit, and bug fixes discovered by new warnings.

Compiler flags (configure.ac)

Category Flags
Format strings -Wformat, -Wformat-nonliteral, -Wformat-security
Code quality -Wextra, -Wundef, -Wfloat-equal, -Wredundant-decls
Safety -Wnull-dereference, -Wpointer-arith, -Wimplicit-function-declaration
Hardening -fstack-protector-strong, -fno-common, -D_FORTIFY_SOURCE=2
GCC-specific -Wlogical-op, -Wduplicated-cond, -Wduplicated-branches, -Wstringop-overflow, -Warray-bounds=2, -Walloc-zero
Suppressions -Wno-unused-parameter, -Wno-missing-field-initializers, -Wno-sign-compare, -Wno-cast-function-type

Format annotations

Added G_GNUC_PRINTF to variadic functions in ui.h, log.h, http_common.h — enables compile-time format string validation.

Security fix

iq.c: User-controlled error_message from XMPP stanza passed directly as format string to cons_show_error() — now wrapped with "%s".

Ran terminal command: wc -l check-cwe134.sh && head -30 check-cwe134.sh

CWE-134 script (check-cwe134.sh)

Replaced the old regex-based heuristic scanner with a two-stage approach:

  1. Attribute audit — verifies that all variadic printf-like wrappers (cons_show, win_println, log_*, etc.) have G_GNUC_PRINTF / __attribute__((format)) annotations. Without the annotation, gcc -Wformat-nonliteral silently ignores format misuse in those functions.
  2. Compile-time check — relies on gcc -Wformat=2 -Werror (now enabled in configure.ac) to catch actual format string violations at build time.

This eliminates false positives from the old grep-based patterns and catches real issues the old script missed (e.g. the iq.c security fix above was found this way).

Bug fixes found by new warnings

File Bug Flag
chatlog.c Non-MUCPM path passed resourcepart instead of NULL as from_jid -Wduplicated-branches
rosterwin.c Two identical branches in contact display (×2) -Wduplicated-branches
omemo.c Redundant branch — else duplicated if body -Wduplicated-branches
console.c Pointer compared with > 0 instead of != NULL -Wextra
stanza.c pri_str/idle_str buffers too small for INT_MIN (10 → 12 bytes) -Wformat-truncation

Format mismatch fixes

File Issue
chatwin.c Jid* passed to %s instead of char*
connection.c %x for long flags → %lx
cmd_funcs.c %d for size_t%zu; gpointer(char*) cast
cmd_defs.c %d for g_list_length() (guint) → %u
iq.c from_jid->barejidfrom_jid->fulljid (matching intent)
console.c, mucwin.c, privwin.c, account.c, omemo.c, presence.c gpointer(char*) casts for %s

Cleanup

File Change
form.c, database.c, muc.c/h, common.c, xmpp.h const qualifiers added
vcard.c, files.c NULL guards added
ui.h, connection.h, omemo.h Removed stale/duplicate declarations
database.c Fixed clang-format alignment
api.c Fixed broken log_warning("%s", "msg %s", var) — extra wrapper caused args to be ignored
cmd_funcs.c Removed unused intval argument from cons_show()
test_common.c Fixed format mismatch in test

Closes #85

### Summary Compiler flags hardening, CWE-134 format string audit, and bug fixes discovered by new warnings. ### Compiler flags (configure.ac) | Category | Flags | |----------|-------| | Format strings | `-Wformat`, `-Wformat-nonliteral`, `-Wformat-security` | | Code quality | `-Wextra`, `-Wundef`, `-Wfloat-equal`, `-Wredundant-decls` | | Safety | `-Wnull-dereference`, `-Wpointer-arith`, `-Wimplicit-function-declaration` | | Hardening | `-fstack-protector-strong`, `-fno-common`, `-D_FORTIFY_SOURCE=2` | | GCC-specific | `-Wlogical-op`, `-Wduplicated-cond`, `-Wduplicated-branches`, `-Wstringop-overflow`, `-Warray-bounds=2`, `-Walloc-zero` | | Suppressions | `-Wno-unused-parameter`, `-Wno-missing-field-initializers`, `-Wno-sign-compare`, `-Wno-cast-function-type` | ### Format annotations Added `G_GNUC_PRINTF` to variadic functions in `ui.h`, `log.h`, `http_common.h` — enables compile-time format string validation. ### Security fix `iq.c`: User-controlled `error_message` from XMPP stanza passed directly as format string to `cons_show_error()` — now wrapped with `"%s"`. Ran terminal command: wc -l check-cwe134.sh && head -30 check-cwe134.sh ### CWE-134 script (check-cwe134.sh) Replaced the old regex-based heuristic scanner with a two-stage approach: 1. **Attribute audit** — verifies that all variadic printf-like wrappers (`cons_show`, `win_println`, `log_*`, etc.) have `G_GNUC_PRINTF` / `__attribute__((format))` annotations. Without the annotation, `gcc -Wformat-nonliteral` silently ignores format misuse in those functions. 2. **Compile-time check** — relies on `gcc -Wformat=2 -Werror` (now enabled in configure.ac) to catch actual format string violations at build time. This eliminates false positives from the old grep-based patterns and catches real issues the old script missed (e.g. the `iq.c` security fix above was found this way). ### Bug fixes found by new warnings | File | Bug | Flag | |------|-----|------| | `chatlog.c` | Non-MUCPM path passed `resourcepart` instead of `NULL` as `from_jid` | `-Wduplicated-branches` | | `rosterwin.c` | Two identical branches in contact display (×2) | `-Wduplicated-branches` | | `omemo.c` | Redundant branch — `else` duplicated `if` body | `-Wduplicated-branches` | | `console.c` | Pointer compared with `> 0` instead of `!= NULL` | `-Wextra` | | `stanza.c` | `pri_str`/`idle_str` buffers too small for `INT_MIN` (10 → 12 bytes) | `-Wformat-truncation` | ### Format mismatch fixes | File | Issue | |------|-------| | `chatwin.c` | `Jid*` passed to `%s` instead of `char*` | | `connection.c` | `%x` for `long` flags → `%lx` | | cmd_funcs.c | `%d` for `size_t` → `%zu`; `gpointer` → `(char*)` cast | | `cmd_defs.c` | `%d` for `g_list_length()` (`guint`) → `%u` | | `iq.c` | `from_jid->barejid` → `from_jid->fulljid` (matching intent) | | `console.c`, `mucwin.c`, `privwin.c`, `account.c`, `omemo.c`, `presence.c` | `gpointer` → `(char*)` casts for `%s` | ### Cleanup | File | Change | |------|--------| | `form.c`, database.c, `muc.c/h`, `common.c`, `xmpp.h` | `const` qualifiers added | | `vcard.c`, `files.c` | NULL guards added | | `ui.h`, `connection.h`, `omemo.h` | Removed stale/duplicate declarations | | database.c | Fixed `clang-format` alignment | | `api.c` | Fixed broken `log_warning("%s", "msg %s", var)` — extra wrapper caused args to be ignored | | cmd_funcs.c | Removed unused `intval` argument from `cons_show()` | | `test_common.c` | Fixed format mismatch in test | Closes #85
jabber.developer2 added 1 commit 2026-03-03 13:34:46 +00:00
fix: CWE-134 format string vulnerability audit
All checks were successful
CI Code / Check spelling (pull_request) Successful in 21s
CI Code / Check coding style (pull_request) Successful in 38s
CI Code / Linux (ubuntu) (pull_request) Successful in 6m25s
CI Code / Linux (debian) (pull_request) Successful in 9m11s
CI Code / Code Coverage (pull_request) Successful in 9m13s
CI Code / Linux (arch) (pull_request) Successful in 11m27s
92953099e1
Security:
- Fix CWE-134 in iq.c: user-controlled string passed as format arg

Annotations:
- Add G_GNUC_PRINTF to variadic functions in ui.h and log.h

Compiler flags (configure.ac):
- Add -Wformat -Wformat-nonliteral -Wno-format-zero-length

Format mismatch fixes:
- chatwin.c: Jid* instead of char* for %s
- connection.c: %x -> %lx for long flags
- cmd_funcs.c: %d -> %zu for MB_CUR_MAX/MB_LEN_MAX (size_t)
- cmd_funcs.c: cast gpointer to (char*) for %s
- cmd_defs.c: %d -> %u for g_list_length() (guint)
- iq.c: from_jid->barejid -> from_jid->fulljid
- console.c, mucwin.c, privwin.c, account.c, omemo.c, presence.c:
  gpointer -> (char*) casts for %s

Bug fixes:
- api.c: broken log_warning() calls with extra format arg
- cmd_funcs.c: remove unused arg from cons_show()

Tooling:
- Expand check-cwe134.sh detection patterns
jabber.developer2 changed title from fix: CWE-134 format string vulnerability audit to WIP: fix: CWE-134 format string vulnerability audit 2026-03-03 15:18:03 +00:00
jabber.developer2 added 2 commits 2026-03-04 19:19:30 +00:00
configure.ac:
- Replace basic -Wformat/-Wformat-nonliteral with -Wformat=2
- Add -Wextra, -Wnull-dereference, -Wpointer-arith, -Wimplicit-function-declaration
- Add -fstack-protector-strong, -fno-common, -D_FORTIFY_SOURCE=2
- Add GCC-specific flags via AC_COMPILE_IFELSE: -Wlogical-op, -Wduplicated-cond,
  -Wduplicated-branches, -Wstringop-overflow
- Add linker hardening via AC_LINK_IFELSE: -Wl,-z,relro -Wl,-z,now
- Suppress noisy -Wextra sub-warnings: -Wno-unused-parameter,
  -Wno-missing-field-initializers, -Wno-sign-compare, -Wno-cast-function-type
- Remove AM_CFLAGS/CFLAGS duplication line

check-cwe134.sh:
- Reduce from 5 checks to 2 (checks 1-3 are now redundant with -Wformat=2)
- Check 1: verify known wrappers have G_GNUC_PRINTF attribute
- Check 2: auto-detect unannotated variadic printf-like functions

Bug fixes found by -Wduplicated-branches:
- chatlog.c: non-MUCPM redact path passed resourcepart instead of NULL
- rosterwin.c: two instances of if/else with identical branches in roster count
- omemo.c: redundant else-if branch in omemo_automatic_start

Other fixes for new warnings:
- console.c: pointer compared to integer 0 instead of NULL (2 instances)
- vcard.c: NULL guard for filename before g_file_set_contents
- files.c: refactor to early return, eliminating NULL logfile path
- database.c: const-correctness for type, query, sort variables
- form.c/xmpp.h: const-correctness for form_set_value parameter
- muc.c/muc.h: remove meaningless top-level const on return type
- common.c: const-correctness for URL string literal
- xmpp/omemo.c: scope block for declarations after goto, move from decl
  before goto, replace goto with direct return
- http_common.h: add G_GNUC_PRINTF attributes for http_print_transfer*
- test_common.c: add currb NULL check to silence -Wnull-dereference
Add low-impact warning flags, fix warnings found by new flags
Some checks failed
CI Code / Check spelling (pull_request) Successful in 22s
CI Code / Check coding style (pull_request) Failing after 32s
CI Code / Linux (ubuntu) (pull_request) Successful in 6m20s
CI Code / Linux (arch) (pull_request) Successful in 6m31s
CI Code / Code Coverage (pull_request) Successful in 8m51s
CI Code / Linux (debian) (pull_request) Successful in 9m6s
60e8b17228
Add compiler flags: -Wundef, -Wfloat-equal, -Wredundant-decls, -Walloc-zero.

Fix -Wredundant-decls warnings:
- Remove stale cons_show_desktop_prefs declaration (ui.h)
- Remove duplicate connection_set_priority prototype (connection.h)
- Remove stale omemo_devicelist_configure_and_request declaration (omemo.h)

Fix potential buffer overflow in stanza.c:
- Increase pri_str and idle_str buffers from 10 to 12 bytes
  (INT_MIN = -2147483648 needs 12 bytes including NUL)
jabber.developer2 force-pushed fix/cwe-134-format-string-audit from 60e8b17228 to 4ef3bc8e46 2026-03-04 19:28:29 +00:00 Compare
jabber.developer2 changed title from WIP: fix: CWE-134 format string vulnerability audit to WIP: fix: CWE-134 format string vulnerability and compiler flags audit 2026-03-04 19:42:13 +00:00
jabber.developer2 changed title from WIP: fix: CWE-134 format string vulnerability and compiler flags audit to fix: CWE-134 format string vulnerability and compiler flags audit 2026-03-05 10:35:22 +00:00
jabber.developer requested changes 2026-03-05 17:32:45 +00:00
jabber.developer left a comment
Owner

Really nice change overall. I have some question marks in certain places + wording of commit message ( (INT_MIN = -2147483648 needs 12 bytes including NUL) (instead of NULL-terminator) slightly confused me. Nonetheless, great job. Just fix the minor issues that were pointed out and it's ready to merge.

Really nice change overall. I have some question marks in certain places + wording of commit message ( `(INT_MIN = -2147483648 needs 12 bytes including NUL)` (instead of `NULL-terminator`) slightly confused me. Nonetheless, great job. Just fix the minor issues that were pointed out and it's ready to merge.
check-cwe134.sh Outdated
@@ -36,0 +75,4 @@
# Find variadic declarations with a const char* parameter followed by ...)
# that do NOT have a format attribute on the preceding line
NEW_ISSUES=$(grep -B1 -rn --include="*.h" \
'const char\s*\*.*,\s*\.\.\.)' "$DIR" 2>/dev/null \

What about gchar?

What about `gchar`?
Author
Collaborator

added

added
jabber.developer marked this conversation as resolved
@@ -984,3 +984,3 @@
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", itemcount);
} else {
} else if (itemcount > 0) {

It doesn't make sense to do it this way. Condition is still duplicated.

Something like this would be more concise.

if (itemcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) 
It doesn't make sense to do it this way. Condition is still duplicated. Something like this would be more concise. ```c if (itemcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) ```
Author
Collaborator

corrected

corrected
jabber.developer marked this conversation as resolved
@@ -1029,3 +1029,3 @@
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", itemcount);
} else {
} else if (itemcount > 0) {

same as above

same as above
Author
Collaborator

corrected

corrected
jabber.developer marked this conversation as resolved
src/xmpp/omemo.c Outdated
@@ -529,3 +523,1 @@
for (device = xmpp_stanza_get_children(list); device != NULL; device = xmpp_stanza_get_next(device)) {
if (g_strcmp0(xmpp_stanza_get_name(device), "device") != 0) {
continue;
/* New scope to keep declarations after goto out above */

Again comment style issue. Also I do not see a point in this scope

Again comment style issue. Also I do not see a point in this scope
Author
Collaborator

Refactored

Refactored
jabber.developer marked this conversation as resolved
@@ -516,3 +516,3 @@
GSList* currb = b;
while (curra) {
while (curra && currb) {

Condition above if (g_slist_length(a) != g_slist_length(b)) { should normally prevent this scenario. It would be strange if one list is going to finish earlier than another one.

Condition above ` if (g_slist_length(a) != g_slist_length(b)) {` should normally prevent this scenario. It would be strange if one list is going to finish earlier than another one.
Author
Collaborator

original code invoke error: potential null pointer dereference [-Werror=null-dereference]
int bval = GPOINTER_TO_INT(currb->data);

original code invoke error: potential null pointer dereference [-Werror=null-dereference] int bval = GPOINTER_TO_INT(currb->data);

I guess we can call is a defensive programming.

I guess we can call is a defensive programming.
jabber.developer marked this conversation as resolved
jabber.developer2 added 1 commit 2026-03-07 10:09:52 +00:00
fix: address PR #98 review feedback
All checks were successful
CI Code / Check spelling (pull_request) Successful in 17s
CI Code / Check coding style (pull_request) Successful in 33s
CI Code / Code Coverage (pull_request) Successful in 4m49s
CI Code / Linux (ubuntu) (pull_request) Successful in 6m14s
CI Code / Linux (debian) (pull_request) Successful in 6m15s
CI Code / Linux (arch) (pull_request) Successful in 6m33s
d103b32025
- check-cwe134.sh: match gchar* in addition to char* in variadic pattern
- rosterwin.c: merge duplicated if/else into single condition
- omemo.c: remove unnecessary scope block and goto, use early return
jabber.developer2 force-pushed fix/cwe-134-format-string-audit from d103b32025 to 872b2bfeac 2026-03-07 10:26:12 +00:00 Compare
Author
Collaborator

suggested squash commit:

fix: CWE-134 format string audit and compiler hardening

Security:

  • Fix CWE-134 in iq.c: user-controlled string passed as format argument
  • Add G_GNUC_PRINTF annotations to all variadic printf-like wrappers
    in ui.h, log.h and http_common.h

Compiler flags (configure.ac):

  • Replace basic -Wformat/-Wformat-nonliteral with -Wformat=2
  • Add -Wextra, -Wnull-dereference, -Wpointer-arith,
    -Wimplicit-function-declaration, -Wundef, -Wfloat-equal,
    -Wredundant-decls, -Walloc-zero
  • Add -fstack-protector-strong, -fno-common, -D_FORTIFY_SOURCE=2
  • Add GCC-specific flags via AC_COMPILE_IFELSE: -Wlogical-op,
    -Wduplicated-cond, -Wduplicated-branches, -Wstringop-overflow,
    -Warray-bounds=2
  • Suppress noisy -Wextra sub-warnings: -Wno-unused-parameter,
    -Wno-missing-field-initializers, -Wno-sign-compare,
    -Wno-cast-function-type
  • Remove AM_CFLAGS/CFLAGS duplication

Bug fixes found by new warnings:

  • chatlog.c: non-MUCPM redact path passed resourcepart instead of NULL
  • rosterwin.c: merge duplicated if/else branches into single condition
  • omemo.c: redundant else-if in omemo_automatic_start; remove
    unnecessary scope block and goto, use early return
  • console.c: pointer compared to integer 0 instead of NULL
  • stanza.c: increase pri_str/idle_str buffers from 10 to 12 bytes
    (INT_MIN = -2147483648 needs 12 bytes including NUL)
  • vcard.c: NULL guard for filename before g_file_set_contents
  • api.c: broken log_warning() calls with extra format argument

Format mismatch fixes:

  • chatwin.c: Jid* → char* for %s
  • connection.c: %x → %lx for long flags
  • cmd_funcs.c: %d → %zu for size_t; cast gpointer to char* for %s
  • cmd_defs.c: %d → %u for g_list_length() return (guint)
  • iq.c: barejid → fulljid for from_jid
  • console.c, mucwin.c, privwin.c, account.c, omemo.c, presence.c:
    gpointer → (char*) casts for %s

Const-correctness and cleanup:

  • database.c: const for type, query, sort variables
  • form.c/xmpp.h: const for form_set_value parameter
  • files.c: refactor to early return, eliminating NULL logfile path
  • muc.c/muc.h: remove meaningless top-level const on return type
  • common.c: const for URL string literal
  • Remove stale declarations: cons_show_desktop_prefs (ui.h),
    connection_set_priority (connection.h),
    omemo_devicelist_configure_and_request (omemo.h)
  • test_common.c: add currb NULL check to silence -Wnull-dereference

Tooling (check-cwe134.sh):

  • Reduce from 5 checks to 2 (checks 1-3 redundant with -Wformat=2)
  • Check 1: verify known wrappers have G_GNUC_PRINTF attribute
  • Check 2: auto-detect unannotated variadic printf-like functions
  • Match both const char* and const gchar* in variadic patterns
suggested squash commit: fix: CWE-134 format string audit and compiler hardening Security: - Fix CWE-134 in iq.c: user-controlled string passed as format argument - Add G_GNUC_PRINTF annotations to all variadic printf-like wrappers in ui.h, log.h and http_common.h Compiler flags (configure.ac): - Replace basic -Wformat/-Wformat-nonliteral with -Wformat=2 - Add -Wextra, -Wnull-dereference, -Wpointer-arith, -Wimplicit-function-declaration, -Wundef, -Wfloat-equal, -Wredundant-decls, -Walloc-zero - Add -fstack-protector-strong, -fno-common, -D_FORTIFY_SOURCE=2 - Add GCC-specific flags via AC_COMPILE_IFELSE: -Wlogical-op, -Wduplicated-cond, -Wduplicated-branches, -Wstringop-overflow, -Warray-bounds=2 - Suppress noisy -Wextra sub-warnings: -Wno-unused-parameter, -Wno-missing-field-initializers, -Wno-sign-compare, -Wno-cast-function-type - Remove AM_CFLAGS/CFLAGS duplication Bug fixes found by new warnings: - chatlog.c: non-MUCPM redact path passed resourcepart instead of NULL - rosterwin.c: merge duplicated if/else branches into single condition - omemo.c: redundant else-if in omemo_automatic_start; remove unnecessary scope block and goto, use early return - console.c: pointer compared to integer 0 instead of NULL - stanza.c: increase pri_str/idle_str buffers from 10 to 12 bytes (INT_MIN = -2147483648 needs 12 bytes including NUL) - vcard.c: NULL guard for filename before g_file_set_contents - api.c: broken log_warning() calls with extra format argument Format mismatch fixes: - chatwin.c: Jid* → char* for %s - connection.c: %x → %lx for long flags - cmd_funcs.c: %d → %zu for size_t; cast gpointer to char* for %s - cmd_defs.c: %d → %u for g_list_length() return (guint) - iq.c: barejid → fulljid for from_jid - console.c, mucwin.c, privwin.c, account.c, omemo.c, presence.c: gpointer → (char*) casts for %s Const-correctness and cleanup: - database.c: const for type, query, sort variables - form.c/xmpp.h: const for form_set_value parameter - files.c: refactor to early return, eliminating NULL logfile path - muc.c/muc.h: remove meaningless top-level const on return type - common.c: const for URL string literal - Remove stale declarations: cons_show_desktop_prefs (ui.h), connection_set_priority (connection.h), omemo_devicelist_configure_and_request (omemo.h) - test_common.c: add currb NULL check to silence -Wnull-dereference Tooling (check-cwe134.sh): - Reduce from 5 checks to 2 (checks 1-3 redundant with -Wformat=2) - Check 1: verify known wrappers have G_GNUC_PRINTF attribute - Check 2: auto-detect unannotated variadic printf-like functions - Match both const char* and const gchar* in variadic patterns
jabber.developer2 force-pushed fix/cwe-134-format-string-audit from 872b2bfeac to d081993c90 2026-03-07 10:32:14 +00:00 Compare
jabber.developer manually merged commit 9ec01fa8cc into master 2026-03-07 10:57:24 +00:00
Sign in to join this conversation.
No description provided.