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
32 changed files with 255 additions and 169 deletions
Showing only changes of commit d081993c90 - Show all commits

View File

@@ -72,13 +72,13 @@ echo ""
KNOWN_RE=$(IFS="|"; echo "${REQUIRED_ATTRIBUTED[*]}") KNOWN_RE=$(IFS="|"; echo "${REQUIRED_ATTRIBUTED[*]}")
# Find variadic declarations with a const char* parameter followed by ...) # Find variadic declarations with a const char* / gchar* parameter followed by ...)
# that do NOT have a format attribute on the preceding line # that do NOT have a format attribute on the preceding line
NEW_ISSUES=$(grep -B1 -rn --include="*.h" \ NEW_ISSUES=$(grep -B1 -rn --include="*.h" \
'const char\s*\*.*,\s*\.\.\.)' "$DIR" 2>/dev/null \ 'const g\?char\s*\*.*,\s*\.\.\.)' "$DIR" 2>/dev/null \
jabber.developer marked this conversation as resolved Outdated

What about gchar?

What about `gchar`?

added

added
| awk ' | awk '
/format\(printf|G_GNUC_PRINTF/ { skip=1; next } /format\(printf|G_GNUC_PRINTF/ { skip=1; next }
/const char.*,.*\.\.\.\)/ { /const g?char.*,.*\.\.\.\)/ {
if (skip) { skip=0; next } if (skip) { skip=0; next }
print print
} }

View File

@@ -981,9 +981,7 @@ _rosterwin_unsubscribed_header(ProfLayoutSplit* layout, GList* wins)
auto_gchar gchar* countpref = prefs_get_string(PREF_ROSTER_COUNT); auto_gchar gchar* countpref = prefs_get_string(PREF_ROSTER_COUNT);
if (g_strcmp0(countpref, "items") == 0) { if (g_strcmp0(countpref, "items") == 0) {
int itemcount = g_list_length(wins); int itemcount = g_list_length(wins);
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) { if (itemcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", itemcount);
} else if (itemcount > 0) {
g_string_append_printf(header, " (%d)", itemcount); g_string_append_printf(header, " (%d)", itemcount);
} }
jabber.developer marked this conversation as resolved Outdated

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

corrected

corrected
} else if (g_strcmp0(countpref, "unread") == 0) { } else if (g_strcmp0(countpref, "unread") == 0) {
@@ -994,9 +992,7 @@ _rosterwin_unsubscribed_header(ProfLayoutSplit* layout, GList* wins)
unreadcount += chatwin->unread; unreadcount += chatwin->unread;
curr = g_list_next(curr); curr = g_list_next(curr);
} }
if (unreadcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) { if (unreadcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", unreadcount);
} else if (unreadcount > 0) {
g_string_append_printf(header, " (%d)", unreadcount); g_string_append_printf(header, " (%d)", unreadcount);
} }
} }
@@ -1026,9 +1022,7 @@ _rosterwin_contacts_header(ProfLayoutSplit* layout, const char* const title, GSL
auto_gchar gchar* countpref = prefs_get_string(PREF_ROSTER_COUNT); auto_gchar gchar* countpref = prefs_get_string(PREF_ROSTER_COUNT);
if (g_strcmp0(countpref, "items") == 0) { if (g_strcmp0(countpref, "items") == 0) {
int itemcount = g_slist_length(contacts); int itemcount = g_slist_length(contacts);
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) { if (itemcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", itemcount);
} else if (itemcount > 0) {
g_string_append_printf(header, " (%d)", itemcount); g_string_append_printf(header, " (%d)", itemcount);
} }
} else if (g_strcmp0(countpref, "unread") == 0) { } else if (g_strcmp0(countpref, "unread") == 0) {
@@ -1043,9 +1037,7 @@ _rosterwin_contacts_header(ProfLayoutSplit* layout, const char* const title, GSL
} }
curr = g_slist_next(curr); curr = g_slist_next(curr);
} }
if (unreadcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) { if (unreadcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", unreadcount);
} else if (unreadcount > 0) {
g_string_append_printf(header, " (%d)", unreadcount); g_string_append_printf(header, " (%d)", unreadcount);
} }
} }
@@ -1136,9 +1128,7 @@ _rosterwin_private_header(ProfLayoutSplit* layout, GList* privs)
auto_gchar gchar* countpref = prefs_get_string(PREF_ROSTER_COUNT); auto_gchar gchar* countpref = prefs_get_string(PREF_ROSTER_COUNT);
if (g_strcmp0(countpref, "items") == 0) { if (g_strcmp0(countpref, "items") == 0) {
int itemcount = g_list_length(privs); int itemcount = g_list_length(privs);
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) { if (itemcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(title_str, " (%d)", itemcount);
} else if (itemcount > 0) {
g_string_append_printf(title_str, " (%d)", itemcount); g_string_append_printf(title_str, " (%d)", itemcount);
} }
} else if (g_strcmp0(countpref, "unread") == 0) { } else if (g_strcmp0(countpref, "unread") == 0) {
@@ -1149,9 +1139,7 @@ _rosterwin_private_header(ProfLayoutSplit* layout, GList* privs)
unreadcount += privwin->unread; unreadcount += privwin->unread;
curr = g_list_next(curr); curr = g_list_next(curr);
} }
if (unreadcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) { if (unreadcount != 0 || prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(title_str, " (%d)", unreadcount);
} else if (unreadcount > 0) {
g_string_append_printf(title_str, " (%d)", unreadcount); g_string_append_printf(title_str, " (%d)", unreadcount);
} }
} }

View File

@@ -517,11 +517,10 @@ _omemo_receive_devicelist(xmpp_stanza_t* const stanza, void* const userdata)
log_warning("[OMEMO] User %s has a non 'current' device item list: %s.", from, xmpp_stanza_get_id(first)); log_warning("[OMEMO] User %s has a non 'current' device item list: %s.", from, xmpp_stanza_get_id(first));
item = first; item = first;
} else { } else {
goto out; omemo_set_device_list(from, device_list);
return 1;
} }
jabber.developer marked this conversation as resolved Outdated

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

Refactored

Refactored
/* New scope to keep declarations after goto out above */
{
xmpp_stanza_t* list = xmpp_stanza_get_child_by_ns(item, STANZA_NS_OMEMO); xmpp_stanza_t* list = xmpp_stanza_get_child_by_ns(item, STANZA_NS_OMEMO);
if (!list) { if (!list) {
return 1; return 1;
@@ -540,9 +539,7 @@ _omemo_receive_devicelist(xmpp_stanza_t* const stanza, void* const userdata)
log_error("[OMEMO] received device without ID"); log_error("[OMEMO] received device without ID");
} }
} }
}
out:
omemo_set_device_list(from, device_list); omemo_set_device_list(from, device_list);
return 1; return 1;