devs/cproof
1
2
Fork 1
Code Issues 43 Pull Requests 9 Actions Packages Projects 1 Releases Wiki Activity

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
Conversation 2 Commits 4 Files Changed 32 +247 -145
29 changed files with 247 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit bb6d29a060 - Show all commits

255
check-cwe134.sh
View File

@@ -1,16 +1,14 @@
#!/bin/bash
# check-cwe134.sh - Static analysis for CWE-134 format string vulnerabilities
# check-cwe134.sh - Verify __attribute__((format)) on printf-like wrappers
#
# This script detects potentially unsafe usage of format string functions
# where user-controlled data may be passed without "%s" wrapper.
# CWE-134 format string vulnerabilities are caught at compile time by
# -Wformat=2 (includes -Wformat-security + -Wformat-nonliteral), BUT only
# for functions annotated with __attribute__((format(printf, N, M))) or
# G_GNUC_PRINTF(N, M).
#
# Checks performed:
# 1. Direct variable passed as format string: cons_show(var);
# 2. Variable passed as format arg through multi-arg wrappers:
# win_println(win, theme, ch, var); (no format specifiers in var position)
# 3. GString->str passed as format argument without "%s"
# 4. Presence of __attribute__((format)) on printf-like wrapper declarations
# 5. Format string mismatch: more arguments than format specifiers
# This script ensures every variadic function whose last fixed parameter
# looks like a format string has the annotation. Without it, the compiler
# silently ignores format misuse.
#
# Usage: ./check-cwe134.sh [directory]
@@ -18,167 +16,15 @@ set -e
DIR="${1:-src}"
echo "=== CWE-134 Format String Vulnerability Check ==="
echo "=== CWE-134: format attribute audit ==="
echo "Scanning: $DIR"
echo ""
# Functions that accept format strings (direct: first arg is format)
DIRECT_FORMAT_FUNCS="cons_show|cons_debug|cons_show_error|log_info|log_error|log_warning|log_debug"
# Functions where format string is not the first arg (multi-arg wrappers)
# win_print(win, theme, char, FORMAT, ...)
# win_println(win, theme, char, FORMAT, ...)
# win_println_indent(win, pad, FORMAT, ...)
# win_append(win, theme, FORMAT, ...)
# win_appendln(win, theme, FORMAT, ...)
# win_append_highlight(win, theme, FORMAT, ...)
# win_appendln_highlight(win, theme, FORMAT, ...)
# win_command_exec_error(win, cmd, FORMAT, ...)
MULTI_ARG_FORMAT_FUNCS="win_print|win_println|win_println_indent|win_append|win_appendln|win_append_highlight|win_appendln_highlight|win_command_exec_error"
ALL_FORMAT_FUNCS="$DIRECT_FORMAT_FUNCS|$MULTI_ARG_FORMAT_FUNCS"
ERRORS=0
# ---------------------------------------------------------------------------
# Check 1: Direct format functions called with single variable (no format string)
# ---------------------------------------------------------------------------
echo "Check 1: Direct variable as format string..."
echo ""
# Pattern: func(identifier) or func(identifier->member) or func(identifier[index])
RESULTS=$(grep -rn --include="*.c" -P "($DIRECT_FORMAT_FUNCS)\s*\(\s*[a-zA-Z_][a-zA-Z0-9_]*(\s*->\s*\w+|\s*\[\s*\w+\s*\])?\s*\)\s*;" "$DIR" 2>/dev/null || true)
# Filter out function definitions, declarations, and safe api_* wrappers
RESULTS=$(echo "$RESULTS" | grep -v "const char\|void \|^[^:]*:[0-9]*:[a-z_]*(\|api_cons_show\|api_log_" || true)
if [ -n "$RESULTS" ]; then
echo "❌ POTENTIAL CWE-134 VULNERABILITIES FOUND (direct format):"
echo ""
echo "$RESULTS"
echo ""
ERRORS=$((ERRORS + $(echo "$RESULTS" | wc -l)))
else
echo "✅ No direct format string issues found."
fi
# ---------------------------------------------------------------------------
# Check 2: Multi-arg wrappers where format position has a variable (not a string literal)
# ---------------------------------------------------------------------------
echo ""
echo "Check 2: Multi-arg wrappers with variable as format argument..."
echo ""
# For win_print(win, theme, char, FORMAT, ...) and win_println(win, theme, char, FORMAT, ...):
# FORMAT is the 4th argument
MULTI4_RESULTS=$(grep -rn --include="*.c" -P "(win_print|win_println)\s*\(" "$DIR" 2>/dev/null | \
grep -v "win_println_indent\|win_println_va" | \
perl -ne '
if (/((win_print|win_println)\s*\()/) {
my $line = $_;
my $call_start = index($line, $1) + length($1);
my $rest = substr($line, $call_start);
my $depth = 0; my $commas = 0; my $i = 0;
for ($i = 0; $i < length($rest) && $commas < 3; $i++) {
my $c = substr($rest, $i, 1);
if ($c eq "(") { $depth++; }
elsif ($c eq ")") { $depth--; last if $depth < 0; }
elsif ($c eq "," && $depth == 0) { $commas++; }
}
if ($commas == 3) {
my $fmt_arg = substr($rest, $i);
$fmt_arg =~ s/^\s+//;
if ($fmt_arg =~ /^[a-zA-Z_]/) { print $line; }
}
}
' || true)
MULTI4_RESULTS=$(echo "$MULTI4_RESULTS" | grep -v "^$\|const char.*message\|void " || true)
# For win_command_exec_error(win, cmd, FORMAT, ...):
# FORMAT is the 3rd argument
MULTI_CMD_RESULTS=$(grep -rn --include="*.c" -P "win_command_exec_error\s*\(" "$DIR" 2>/dev/null | \
perl -ne '
if (/(win_command_exec_error\s*\()/) {
my $line = $_;
my $call_start = index($line, $1) + length($1);
my $rest = substr($line, $call_start);
my $depth = 0; my $commas = 0; my $i = 0;
for ($i = 0; $i < length($rest) && $commas < 2; $i++) {
my $c = substr($rest, $i, 1);
if ($c eq "(") { $depth++; }
elsif ($c eq ")") { $depth--; last if $depth < 0; }
elsif ($c eq "," && $depth == 0) { $commas++; }
}
if ($commas == 2) {
my $fmt_arg = substr($rest, $i);
$fmt_arg =~ s/^\s+//;
if ($fmt_arg =~ /^[a-zA-Z_]/) { print $line; }
}
}
' || true)
MULTI_CMD_RESULTS=$(echo "$MULTI_CMD_RESULTS" | grep -v "^$\|const char.*error\|void \|ProfWin\*.*const char\*.*const char\*.*\.\.\." || true)
# For 3-arg format functions: win_println_indent(win, pad, FORMAT, ...)
# win_append(win, theme, FORMAT, ...) etc.
MULTI3_RESULTS=$(grep -rn --include="*.c" -P "(win_println_indent|win_append|win_appendln|win_append_highlight|win_appendln_highlight)\s*\([^,]+,[^,]+,\s*(?!\")\s*[a-zA-Z_][a-zA-Z0-9_]*(\s*->\s*\w+|\s*\[\s*[^\]]+\])?\s*\)\s*;" "$DIR" 2>/dev/null || true)
MULTI3_RESULTS=$(echo "$MULTI3_RESULTS" | grep -v "^$\|const char.*message\|void " || true)
MULTI_RESULTS=""
if [ -n "$MULTI4_RESULTS" ]; then
MULTI_RESULTS="$MULTI4_RESULTS"
fi
if [ -n "$MULTI3_RESULTS" ]; then
if [ -n "$MULTI_RESULTS" ]; then
MULTI_RESULTS="$MULTI_RESULTS
$MULTI3_RESULTS"
else
MULTI_RESULTS="$MULTI3_RESULTS"
fi
fi
if [ -n "$MULTI_CMD_RESULTS" ]; then
if [ -n "$MULTI_RESULTS" ]; then
MULTI_RESULTS="$MULTI_RESULTS
$MULTI_CMD_RESULTS"
else
MULTI_RESULTS="$MULTI_CMD_RESULTS"
fi
fi
if [ -n "$MULTI_RESULTS" ]; then
echo "❌ POTENTIAL CWE-134 VULNERABILITIES FOUND (multi-arg wrappers):"
echo ""
echo "$MULTI_RESULTS"
echo ""
ERRORS=$((ERRORS + $(echo "$MULTI_RESULTS" | wc -l)))
else
echo "✅ No multi-arg format string issues found."
fi
# ---------------------------------------------------------------------------
# Check 3: GString->str passed directly as format argument
# ---------------------------------------------------------------------------
echo ""
echo "Check 3: GString->str passed to format functions..."
GSTRING_RESULTS=$(grep -rn --include="*.c" -P "($ALL_FORMAT_FUNCS)\s*\([^)]*->str\s*\)" "$DIR" 2>/dev/null | grep -v '"%s"' || true)
if [ -n "$GSTRING_RESULTS" ]; then
echo "⚠️ GString->str passed without \"%s\" (review manually):"
echo ""
echo "$GSTRING_RESULTS"
echo ""
fi
# ---------------------------------------------------------------------------
# Check 4: Verify __attribute__((format)) on printf-like wrappers
# ---------------------------------------------------------------------------
echo ""
echo "Check 4: Verifying __attribute__((format)) annotations..."
echo ""
ATTR_ERRORS=0
# Functions that MUST have format attributes (declared in headers)
# --------------------------------------------------------------------- #
# Known printf-like wrappers that MUST have the attribute #
# --------------------------------------------------------------------- #
REQUIRED_ATTRIBUTED=(
"cons_show"
"cons_debug"
@@ -199,39 +45,74 @@ REQUIRED_ATTRIBUTED=(
"win_command_exec_error"
)
echo "Check 1: Known wrappers must have G_GNUC_PRINTF / __attribute__((format))"
echo ""
for func in "${REQUIRED_ATTRIBUTED[@]}"; do
# Check if the function declaration in headers has a preceding format attribute
HAS_ATTR=$(grep -B1 --include="*.h" -rn "void ${func}\s*(" "$DIR" 2>/dev/null | grep -E -c "format\(printf|G_GNUC_PRINTF" || true)
HAS_ATTR=$(grep -B1 --include="*.h" -rn "void ${func}\s*(" "$DIR" 2>/dev/null \
| grep -Ec "format\(printf|G_GNUC_PRINTF" || true)
if [ "$HAS_ATTR" -eq 0 ]; then
echo "❌ Missing __attribute__((format(printf, ...))) for: $func"
ATTR_ERRORS=$((ATTR_ERRORS + 1))
echo " ❌ $func — missing format attribute"
ERRORS=$((ERRORS + 1))
fi
done
if [ "$ATTR_ERRORS" -eq 0 ]; then
echo "✅ All printf-like wrappers have format attributes."
else
echo ""
echo "⚠️ $ATTR_ERRORS function(s) missing format attributes."
echo " Add __attribute__((format(printf, N, M))) before the declaration."
ERRORS=$((ERRORS + ATTR_ERRORS))
if [ "$ERRORS" -eq 0 ]; then
echo " ✅ All known wrappers annotated."
fi
# ---------------------------------------------------------------------------
# Summary
# ---------------------------------------------------------------------------
# --------------------------------------------------------------------- #
# Auto-detect new variadic functions that look like printf wrappers #
# but are NOT in the known list and NOT annotated. #
# Heuristic: declaration has (... const char* ..., ...) and no attribute #
# --------------------------------------------------------------------- #
echo ""
echo "Check 2: Detect unannotated printf-like variadic declarations in headers"
echo ""
KNOWN_RE=$(IFS="|"; echo "${REQUIRED_ATTRIBUTED[*]}")
# 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 \
jabber.developer marked this conversation as resolved Outdated
jabber.developer commented 2026-03-05 17:03:49 +00:00
Outdated
Review
Copy Link

What about gchar?

What about `gchar`?
jabber.developer2 commented 2026-03-07 09:59:59 +00:00
Outdated
Review
Copy Link

added

added
| awk '
/format\(printf|G_GNUC_PRINTF/ { skip=1; next }
/const char.*,.*\.\.\.\)/ {
if (skip) { skip=0; next }
print
}
{ skip=0 }
' \
| grep -E "void\s+\w+\s*\(" \
| grep -Ev "($KNOWN_RE)" \
|| true)
if [ -n "$NEW_ISSUES" ]; then
echo " ⚠️ Possibly unannotated new printf-like functions:"
echo ""
echo "$NEW_ISSUES"
echo ""
NEW_COUNT=$(echo "$NEW_ISSUES" | wc -l)
ERRORS=$((ERRORS + NEW_COUNT))
else
echo " ✅ No unannotated variadic printf-like functions found."
fi
# --------------------------------------------------------------------- #
# Summary #
# --------------------------------------------------------------------- #
echo ""
echo "=== Summary ==="
echo "Critical issues: $ERRORS"
echo "Issues: $ERRORS"
echo ""
if [ "$ERRORS" -gt 0 ]; then
echo "Fix: add the attribute before the declaration in the .h file:"
echo " G_GNUC_PRINTF(N, M) // N = format arg, M = first vararg"
echo " void my_func(ProfWin* w, const char* fmt, ...);"
echo ""
echo "Fix by adding \"%s\" format specifier:"
echo " BAD: cons_show(variable);"
echo " GOOD: cons_show(\"%s\", variable);"
echo ""
echo " BAD: win_println(win, theme, ch, variable);"
echo " GOOD: win_println(win, theme, ch, \"%s\", variable);"
echo "The compiler flag -Wformat=2 will then catch all misuse automatically."
exit 1
fi

34
configure.ac
View File

@@ -385,9 +385,39 @@ AC_CHECK_LIB([util], [forkpty], [AM_CONDITIONAL([HAVE_FORKPTY], [true]) FORKPTY_
AC_SUBST([FORKPTY_LIB])
## Default parameters
AM_CFLAGS="$AM_CFLAGS -Wall -Wformat -Wformat-nonliteral -Wno-format-zero-length -Wno-deprecated-declarations -std=gnu99 -ggdb3"
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 -Wnull-dereference -Wpointer-arith"
AM_CFLAGS="$AM_CFLAGS -Wimplicit-function-declaration"
AM_CFLAGS="$AM_CFLAGS -fstack-protector-strong -fno-common"
AM_CFLAGS="$AM_CFLAGS -D_FORTIFY_SOURCE=2"
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; 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"
AM_LDFLAGS="$AM_LDFLAGS -export-dynamic"
# Linker hardening (RELRO + immediate binding)
saved_LDFLAGS="$LDFLAGS"
for _flag in -Wl,-z,relro -Wl,-z,now; do
AC_MSG_CHECKING([whether linker supports $_flag])
LDFLAGS="$saved_LDFLAGS $_flag"
AC_LINK_IFELSE([AC_LANG_PROGRAM()],
[AC_MSG_RESULT([yes]); AM_LDFLAGS="$AM_LDFLAGS $_flag"],
[AC_MSG_RESULT([no])])
done
LDFLAGS="$saved_LDFLAGS"
AS_IF([test "x$enable_coverage" = xyes],
[AM_CFLAGS="$AM_CFLAGS --coverage -O0"
AM_LDFLAGS="$AM_LDFLAGS --coverage"
@@ -401,7 +431,7 @@ AS_IF([test "x$PLATFORM" = xosx],
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\\\"\""
AM_CFLAGS="$AM_CFLAGS $CFLAGS"
LIBS="$glib_LIBS $gio_LIBS $PTHREAD_LIBS $curl_LIBS $libnotify_LIBS $python_LIBS ${GTK_LIBS} ${SQLITE_LIBS} $LIBS"
AC_SUBST(AM_LDFLAGS)

2
src/chatlog.c
View File

@@ -204,7 +204,7 @@ chat_log_omemo_msg_in(ProfMessage* message)
if (message->type == PROF_MSG_TYPE_MUCPM) {
_chat_log_chat(mybarejid, message->from_jid->barejid, "[redacted]", PROF_IN_LOG, message->timestamp, message->from_jid->resourcepart);
} else {
_chat_log_chat(mybarejid, message->from_jid->barejid, "[redacted]", PROF_IN_LOG, message->timestamp, message->from_jid->resourcepart);
_chat_log_chat(mybarejid, message->from_jid->barejid, "[redacted]", PROF_IN_LOG, message->timestamp, NULL);
}
}
}

2
src/common.c
View File

@@ -383,7 +383,7 @@ utf8_display_len(const char* const str)
char*
release_get_latest(void)
{
char* url = "https://profanity-im.github.io/profanity_version.txt";
const char* url = "https://profanity-im.github.io/profanity_version.txt";
CURL* handle = curl_easy_init();
struct curl_data_t output;

24
src/config/files.c
View File

@@ -114,30 +114,26 @@ gchar*
files_get_log_file(const char* const log_file)
{
auto_gchar gchar* xdg_data = _files_get_xdg_data_home();
GString* logfile;
if (log_file) {
auto_gchar gchar* log_path = g_path_get_dirname(log_file);
if (!create_dir(log_path)) {
log_error("Error while creating directory %s", log_path);
}
logfile = g_string_new(log_file);
} else {
logfile = g_string_new(xdg_data);
g_string_append(logfile, "/profanity/logs/profanity");
if (!prefs_get_boolean(PREF_LOG_SHARED)) {
g_string_append_printf(logfile, "%d", getpid());
}
g_string_append(logfile, ".log");
return g_strdup(log_file);
}
GString* logfile = g_string_new(xdg_data);
g_string_append(logfile, "/profanity/logs/profanity");
if (!prefs_get_boolean(PREF_LOG_SHARED)) {
g_string_append_printf(logfile, "%d", getpid());
}
g_string_append(logfile, ".log");
gchar* result = g_strdup(logfile->str);
g_string_free(logfile, TRUE);
return result;
}

12
src/database.c
View File

@@ -55,7 +55,7 @@
static sqlite3* g_chatlog_database;
static void _add_to_db(ProfMessage* message, char* type, const Jid* const from_jid, const Jid* const to_jid);
static void _add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, const Jid* const to_jid);
static char* _get_db_filename(ProfAccount* account);
static prof_msg_type_t _get_message_type_type(const char* const type);
static prof_enc_t _get_message_enc_type(const char* const encstr);
@@ -167,7 +167,7 @@ log_database_init(ProfAccount* account)
// replace_id is the ID from XEP-0308: Last Message Correction
// replaces_db_id is ID (primary key) of the original message that LMC message corrects/replaces
// replaced_by_db_id is ID (primary key) of the last correcting (LMC) message for the original message
char* query = "CREATE TABLE IF NOT EXISTS `ChatLogs` ("
const char* query = "CREATE TABLE IF NOT EXISTS `ChatLogs` ("
"`id` INTEGER PRIMARY KEY AUTOINCREMENT, "
"`from_jid` TEXT NOT NULL, "
"`to_jid` TEXT NOT NULL, "
@@ -272,7 +272,7 @@ log_database_add_incoming(ProfMessage* message)
}
static void
_log_database_add_outgoing(char* type, const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc)
_log_database_add_outgoing(const char* type, const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc)
{
ProfMessage* msg = message_init();
@@ -379,8 +379,8 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const gchar*
}
// Flip order when querying older pages
gchar* sort1 = from_start ? "ASC" : "DESC";
gchar* sort2 = !flip ? "ASC" : "DESC";
const gchar* sort1 = from_start ? "ASC" : "DESC";
const gchar* sort2 = !flip ? "ASC" : "DESC";
GDateTime* now = g_date_time_new_now_local();
auto_gchar gchar* end_date_fmt = end_time ? g_strdup(end_time) : g_date_time_format_iso8601(now);
auto_sqlite gchar* query = sqlite3_mprintf("SELECT * FROM ("
@@ -499,7 +499,7 @@ _get_message_enc_type(const char* const encstr)
}
static void
_add_to_db(ProfMessage* message, char* type, const Jid* const from_jid, const Jid* const to_jid)
_add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, const Jid* const to_jid)
{
auto_gchar gchar* pref_dblog = prefs_get_string(PREF_DBLOG);
sqlite_int64 original_message_id = -1;

2
src/omemo/omemo.c
View File

@@ -1406,8 +1406,6 @@ omemo_automatic_start(const char* const recipient)
case PROF_OMEMOPOLICY_AUTOMATIC:
if (g_list_find_custom(account->omemo_enabled, recipient, (GCompareFunc)g_strcmp0)) {
result = TRUE;
} else if (g_list_find_custom(account->omemo_disabled, recipient, (GCompareFunc)g_strcmp0)) {
result = FALSE;
} else {
result = FALSE;
}

3
src/tools/http_common.h
View File

@@ -36,9 +36,12 @@
#ifndef TOOLS_HTTP_COMMON_H
#define TOOLS_HTTP_COMMON_H
#include <glib.h>
#include "ui/window.h"
G_GNUC_PRINTF(3, 4)
void http_print_transfer(ProfWin* window, char* id, const char* fmt, ...);
G_GNUC_PRINTF(3, 4)
void http_print_transfer_update(ProfWin* window, char* id, const char* fmt, ...);
#endif

4
src/ui/console.c
View File

@@ -470,7 +470,7 @@ cons_show_wins(gboolean unread)
GSList* curr = window_strings;
while (curr) {
if (g_strstr_len((char*)curr->data, strlen((char*)curr->data), " unread") > 0) {
if (g_strstr_len((char*)curr->data, strlen((char*)curr->data), " unread") != NULL) {
win_println(console, THEME_CMD_WINS_UNREAD, "-", "%s", (char*)curr->data);
} else {
win_println(console, THEME_DEFAULT, "-", "%s", (char*)curr->data);
@@ -491,7 +491,7 @@ cons_show_wins_attention()
GSList* curr = window_strings;
while (curr) {
if (g_strstr_len((char*)curr->data, strlen((char*)curr->data), " unread") > 0) {
if (g_strstr_len((char*)curr->data, strlen((char*)curr->data), " unread") != NULL) {
win_println(console, THEME_CMD_WINS_UNREAD, "-", "%s", (char*)curr->data);
} else {
win_println(console, THEME_DEFAULT, "-", "%s", (char*)curr->data);

4
src/ui/rosterwin.c
View File

@@ -983,7 +983,7 @@ _rosterwin_unsubscribed_header(ProfLayoutSplit* layout, GList* wins)
int itemcount = g_list_length(wins);
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", itemcount);
} else {
} else if (itemcount > 0) {
jabber.developer marked this conversation as resolved Outdated
jabber.developer commented 2026-03-05 17:15:54 +00:00
Outdated
Review
Copy Link

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)) ```
jabber.developer2 commented 2026-03-07 10:00:23 +00:00
Outdated
Review
Copy Link

corrected

corrected
g_string_append_printf(header, " (%d)", itemcount);
}
} else if (g_strcmp0(countpref, "unread") == 0) {
@@ -1028,7 +1028,7 @@ _rosterwin_contacts_header(ProfLayoutSplit* layout, const char* const title, GSL
int itemcount = g_slist_length(contacts);
if (itemcount == 0 && prefs_get_boolean(PREF_ROSTER_COUNT_ZERO)) {
g_string_append_printf(header, " (%d)", itemcount);
} else {
} else if (itemcount > 0) {
jabber.developer marked this conversation as resolved Outdated
jabber.developer commented 2026-03-05 17:16:05 +00:00
Outdated
Review
Copy Link

same as above

same as above
jabber.developer2 commented 2026-03-07 10:00:31 +00:00
Outdated
Review
Copy Link

corrected

corrected
g_string_append_printf(header, " (%d)", itemcount);
}
} else if (g_strcmp0(countpref, "unread") == 0) {

2
src/xmpp/form.c
View File

@@ -458,7 +458,7 @@ form_get_field_type(DataForm* form, const char* const tag)
}
void
form_set_value(DataForm* form, const char* const tag, char* value)
form_set_value(DataForm* form, const char* const tag, const char* value)
{
char* var = g_hash_table_lookup(form->tag_to_var, tag);
if (var) {

2
src/xmpp/muc.c
View File

@@ -427,7 +427,7 @@ muc_rooms(void)
* Return current users nickname for the specified room
* The nickname is owned by the chat room and should not be modified or freed
*/
const char* const
const char*
muc_nick(const char* const room)
{
ChatRoom* chat_room = g_hash_table_lookup(rooms, room);

2
src/xmpp/muc.h
View File

@@ -93,7 +93,7 @@ GList* muc_rooms(void);
void muc_set_features(const char* const room, GSList* features);
const char* const muc_nick(const char* const room);
const char* muc_nick(const char* const room);
char* muc_password(const char* const room);
void muc_nick_change_start(const char* const room, const char* const new_nick);

37
src/xmpp/omemo.c
View File

@@ -353,6 +353,7 @@ omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted)
{
char* plaintext = NULL;
const char* type = xmpp_stanza_get_type(stanza);
const char* from = xmpp_stanza_get_from(stanza);
GList* keys = NULL;
unsigned char* iv_raw = NULL;
unsigned char* payload_raw = NULL;
@@ -434,8 +435,6 @@ omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted)
keys = g_list_append(keys, key);
}
const char* from = xmpp_stanza_get_from(stanza);
plaintext = omemo_on_message_recv(from, sid, iv_raw, iv_len,
keys, payload_raw, payload_len,
g_strcmp0(type, STANZA_TYPE_GROUPCHAT) == 0, trusted);
@@ -471,7 +470,8 @@ _omemo_receive_devicelist(xmpp_stanza_t* const stanza, void* const userdata)
const char* code = xmpp_stanza_get_attribute(error, "code");
if (g_strcmp0(code, "404") == 0) {
goto out;
omemo_set_device_list(from, NULL);
return 1;
}
}
@@ -520,22 +520,25 @@ _omemo_receive_devicelist(xmpp_stanza_t* const stanza, void* const userdata)
goto out;
}
xmpp_stanza_t* list = xmpp_stanza_get_child_by_ns(item, STANZA_NS_OMEMO);
if (!list) {
return 1;
}
xmpp_stanza_t* device;
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 */
jabber.developer marked this conversation as resolved Outdated
jabber.developer commented 2026-03-05 17:22:47 +00:00
Outdated
Review
Copy Link

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
jabber.developer2 commented 2026-03-07 10:00:50 +00:00
Outdated
Review
Copy Link

Refactored

Refactored
{
xmpp_stanza_t* list = xmpp_stanza_get_child_by_ns(item, STANZA_NS_OMEMO);
if (!list) {
return 1;
}
const char* id = xmpp_stanza_get_id(device);
if (id != NULL) {
device_list = g_list_append(device_list, GINT_TO_POINTER(strtoul(id, NULL, 10)));
} else {
log_error("[OMEMO] received device without ID");
xmpp_stanza_t* device;
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;
}
const char* id = xmpp_stanza_get_id(device);
if (id != NULL) {
device_list = g_list_append(device_list, GINT_TO_POINTER(strtoul(id, NULL, 10)));
} else {
log_error("[OMEMO] received device without ID");
}
}
}

7
src/xmpp/vcard.c
View File

@@ -1300,7 +1300,7 @@ _vcard_photo_result(xmpp_stanza_t* const stanza, void* userdata)
return 1;
}
GString* filename;
GString* filename = NULL;
if (!data->filename) {
auto_gchar gchar* path = files_get_data_path(DIR_PHOTOS);
@@ -1340,6 +1340,11 @@ _vcard_photo_result(xmpp_stanza_t* const stanza, void* userdata)
GError* err = NULL;
if (!filename) {
cons_show_error("Unable to determine filename for photo");
return 1;
}
if (g_file_set_contents(filename->str, (gchar*)photo->data, photo->length, &err) == FALSE) {
cons_show_error("Unable to save photo: %s", err->message);
g_error_free(err);

2
src/xmpp/xmpp.h
View File

@@ -305,7 +305,7 @@ char* blocked_ac_find(const char* const search_str, gboolean previous, void* con
void blocked_ac_reset(void);
void form_destroy(DataForm* form);
void form_set_value(DataForm* form, const char* const tag, char* value);
void form_set_value(DataForm* form, const char* const tag, const char* value);
gboolean form_add_unique_value(DataForm* form, const char* const tag, char* value);
void form_add_value(DataForm* form, const char* const tag, char* value);
gboolean form_remove_value(DataForm* form, const char* const tag, char* value);

2
tests/unittests/test_common.c
View File

@@ -515,7 +515,7 @@ _lists_equal(GSList* a, GSList* b)
GSList* curra = a;
GSList* currb = b;
while (curra) {
while (curra && currb) {
jabber.developer marked this conversation as resolved
jabber.developer commented 2026-03-05 17:30:27 +00:00
Review
Copy Link

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.
jabber.developer2 commented 2026-03-07 09:59:41 +00:00
Review
Copy Link

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);
jabber.developer commented 2026-03-07 10:23:58 +00:00
Review
Copy Link

I guess we can call is a defensive programming.

I guess we can call is a defensive programming.
int aval = GPOINTER_TO_INT(curra->data);
int bval = GPOINTER_TO_INT(currb->data);