From 22335ca83e7dfce85014d69827158685f4035621 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Thu, 5 Feb 2026 17:22:04 +0300 Subject: [PATCH] fix(security): prevent CWE-134 format string injection - add "%s" to unsafe cons_show/log_*/win_println calls - add check-cwe134.sh static analysis script - integrate security check into CI pipeline Closes #85 --- .github/workflows/ci-code.yml | 3 ++ check-cwe134.sh | 69 +++++++++++++++++++++++++++++++++++ src/command/cmd_funcs.c | 4 +- src/common.c | 4 +- src/plugins/python_plugins.c | 8 ++-- src/profanity.c | 4 +- src/tools/http_upload.c | 4 +- src/ui/console.c | 2 +- src/ui/core.c | 2 +- src/ui/window.c | 2 +- src/xmpp/message.c | 2 +- src/xmpp/presence.c | 2 +- 12 files changed, 89 insertions(+), 17 deletions(-) create mode 100755 check-cwe134.sh diff --git a/.github/workflows/ci-code.yml b/.github/workflows/ci-code.yml index e4e8dc77..ae9e7297 100644 --- a/.github/workflows/ci-code.yml +++ b/.github/workflows/ci-code.yml @@ -50,6 +50,9 @@ jobs: run: | grep -P 'auto_(char|gchar|gcharv|guchar|jid|sqlite|gfd|FILE)[\w *]*;$' -r src && exit -1 || true + - name: Check CWE-134 format string vulnerabilities + run: ./check-cwe134.sh + - name: Install clang-format run: | sudo apt-get update diff --git a/check-cwe134.sh b/check-cwe134.sh new file mode 100755 index 00000000..4ff6b67f --- /dev/null +++ b/check-cwe134.sh @@ -0,0 +1,69 @@ +#!/bin/bash +# check-cwe134.sh - Static analysis for CWE-134 format string vulnerabilities +# +# This script detects potentially unsafe usage of format string functions +# where user-controlled data may be passed without "%s" wrapper. +# +# Usage: ./check-cwe134.sh [directory] + +set -e + +DIR="${1:-src}" + +echo "=== CWE-134 Format String Vulnerability Check ===" +echo "Scanning: $DIR" +echo "" + +# Functions that accept format strings +FORMAT_FUNCS="cons_show|cons_debug|cons_show_error|log_info|log_error|log_warning|log_debug|win_println|win_print" + +ERRORS=0 + +echo "Checking for unsafe format string usage..." +echo "" + +# Pattern 1: function call with single variable argument (no format string) +# Example: cons_show(variable); - BAD +# Example: cons_show("%s", variable); - OK +# Matches: func(identifier) or func(identifier->member) or func(identifier[index]) +RESULTS=$(grep -rn --include="*.c" -P "($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:" + echo "" + echo "$RESULTS" + echo "" + ERRORS=$(echo "$RESULTS" | wc -l) +else + echo "✅ No obvious CWE-134 issues found." +fi + +# Additional check: GString->str passed directly (not as %s argument) +echo "" +echo "Checking for GString->str passed to format functions..." + +GSTRING_RESULTS=$(grep -rn --include="*.c" -P "($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 + +echo "" +echo "=== Summary ===" +echo "Critical issues: $ERRORS" + +if [ "$ERRORS" -gt 0 ]; then + echo "" + echo "Fix by adding \"%s\" format specifier:" + echo " BAD: cons_show(variable);" + echo " GOOD: cons_show(\"%s\", variable);" + exit 1 +fi + +exit 0 diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 98d01f31..f12c9038 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -4970,8 +4970,8 @@ cmd_sendfile(ProfWin* window, const char* const command, gchar** args) alt_scheme = OMEMO_AESGCM_URL_SCHEME; alt_fragment = _add_omemo_stream(&fd, &fh, &err); if (err != NULL) { - cons_show_error(err); - win_println(window, THEME_ERROR, "-", err); + cons_show_error("%s", err); + win_println(window, THEME_ERROR, "-", "%s", err); goto out; } #endif diff --git a/src/common.c b/src/common.c index 064eb757..69f90900 100644 --- a/src/common.c +++ b/src/common.c @@ -143,7 +143,7 @@ auto_close_gfd(gint* fd) return; if (close(*fd) == EOF) - log_error(g_strerror(errno)); + log_error("%s", g_strerror(errno)); } /** @@ -158,7 +158,7 @@ auto_close_FILE(FILE** fd) return; if (fclose(*fd) == EOF) - log_error(g_strerror(errno)); + log_error("%s", g_strerror(errno)); } static gboolean diff --git a/src/plugins/python_plugins.c b/src/plugins/python_plugins.c index 3b50c961..0e178501 100644 --- a/src/plugins/python_plugins.c +++ b/src/plugins/python_plugins.c @@ -887,8 +887,8 @@ _python_undefined_error(ProfPlugin* plugin, char* hook, char* type) g_string_append(err_msg, hook); g_string_append(err_msg, "(): return value undefined, expected "); g_string_append(err_msg, type); - log_error(err_msg->str); - cons_show_error(err_msg->str); + log_error("%s", err_msg->str); + cons_show_error("%s", err_msg->str); g_string_free(err_msg, TRUE); } @@ -901,8 +901,8 @@ _python_type_error(ProfPlugin* plugin, char* hook, char* type) g_string_append(err_msg, hook); g_string_append(err_msg, "(): incorrect return type, expected "); g_string_append(err_msg, type); - log_error(err_msg->str); - cons_show_error(err_msg->str); + log_error("%s", err_msg->str); + cons_show_error("%s", err_msg->str); g_string_free(err_msg, TRUE); } diff --git a/src/profanity.c b/src/profanity.c index 750f7891..b9b0cc87 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -135,7 +135,7 @@ prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_f */ min_runtime += waittime; } else { - log_error(err_msg); + log_error("%s", err_msg); g_free(err_msg); commands = NULL; } @@ -245,7 +245,7 @@ _init(char* log_level, char* config_file, char* log_file, char* theme_name) if (prof_log_level == PROF_LEVEL_DEBUG) { ProfWin* console = wins_get_console(); win_println(console, THEME_DEFAULT, "-", "Debug mode enabled! Logging to: "); - win_println(console, THEME_DEFAULT, "-", get_log_file_location()); + win_println(console, THEME_DEFAULT, "-", "%s", get_log_file_location()); } session_init(); cmd_init(); diff --git a/src/tools/http_upload.c b/src/tools/http_upload.c index a1c31b27..166797f1 100644 --- a/src/tools/http_upload.c +++ b/src/tools/http_upload.c @@ -311,7 +311,7 @@ http_file_put(void* userdata) } win_update_entry_message(upload->window, upload->put_url, err_msg); } - cons_show_error(err_msg); + cons_show_error("%s", err_msg); } else { if (!upload->cancel) { auto_gchar gchar* status_msg = g_strdup_printf("Uploading '%s': 100%%", upload->filename); @@ -327,7 +327,7 @@ http_file_put(void* userdata) if (!fail_msg) { fail_msg = g_strdup(FALLBACK_MSG); } - cons_show_error(fail_msg); + cons_show_error("%s", fail_msg); } else { switch (upload->window->type) { case WIN_CHAT: diff --git a/src/ui/console.c b/src/ui/console.c index 52c281e4..d1104fa0 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -938,7 +938,7 @@ cons_show_account_list(gchar** accounts) theme_item_t presence_colour = theme_main_presence_attrs(string_from_resource_presence(presence)); win_println(console, presence_colour, "-", "%s", accounts[i]); } else { - cons_show(accounts[i]); + cons_show("%s", accounts[i]); } } cons_show(""); diff --git a/src/ui/core.c b/src/ui/core.c index e61831a0..be77756c 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -445,7 +445,7 @@ ui_handle_error(const char* const err_msg) GString* msg = g_string_new(""); g_string_printf(msg, "Error %s", err_msg); - cons_show_error(msg->str); + cons_show_error("%s", msg->str); g_string_free(msg, TRUE); } diff --git a/src/ui/window.c b/src/ui/window.c index c4b8c92f..a652426c 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -2297,7 +2297,7 @@ void win_handle_command_exec_result_note(ProfWin* window, const char* const type, const char* const value) { assert(window != NULL); - win_println(window, THEME_DEFAULT, "!", value); + win_println(window, THEME_DEFAULT, "!", "%s", value); } void diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 30fb2696..9af7b8c4 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -868,7 +868,7 @@ _handle_error(xmpp_stanza_t* const stanza) g_string_append(log_msg, " error="); g_string_append(log_msg, err_msg); - log_info(log_msg->str); + log_info("%s", log_msg->str); g_string_free(log_msg, TRUE); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 05123d08..8916235a 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -455,7 +455,7 @@ _presence_error_handler(xmpp_stanza_t* const stanza) g_string_append(log_msg, " error="); g_string_append(log_msg, err_msg); - log_info(log_msg->str); + log_info("%s", log_msg->str); g_string_free(log_msg, TRUE);