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/CONTRIBUTING.md b/CONTRIBUTING.md index 87edee02..de170532 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,6 +144,16 @@ scan-build make scan-view ... ``` +### Security checks + +We have a static analyzer `check-cwe134.sh` that detects CWE-134 format string vulnerabilities. It runs automatically in CI but you can also run it locally: + +```bash +./check-cwe134.sh +``` + +This checks for unsafe patterns where data could be passed directly as a format string to functions like `printf`, `cons_show`, etc. Never pass a raw string for formatting; use `"%s"` format specifier instead. + ### Finding typos We include a `.codespellrc` configuration file for `codespell` in the root directory. diff --git a/Makefile.am b/Makefile.am index 1735d1ea..60ff0f33 100644 --- a/Makefile.am +++ b/Makefile.am @@ -185,6 +185,7 @@ functionaltest_sources = \ tests/functionaltests/test_software.c tests/functionaltests/test_software.h \ tests/functionaltests/test_muc.c tests/functionaltests/test_muc.h \ tests/functionaltests/test_disconnect.c tests/functionaltests/test_disconnect.h \ + tests/functionaltests/test_lastactivity.c tests/functionaltests/test_lastactivity.h \ tests/functionaltests/functionaltests.c main_source = src/main.c 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_defs.c b/src/command/cmd_defs.c index ec1e58b3..124749d6 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2825,16 +2825,15 @@ cmd_search_index_any(char* term) int terms_len = g_strv_length(processed_terms); for (int i = 0; i < terms_len; i++) { - GList* index_keys = g_hash_table_get_keys(search_index); - GList* curr = index_keys; - while (curr) { - char* index_entry = g_hash_table_lookup(search_index, curr->data); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, search_index); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* index_entry = (char*)value; if (g_str_match_string(processed_terms[i], index_entry, FALSE)) { - results = g_list_append(results, curr->data); + results = g_list_append(results, key); } - curr = g_list_next(curr); } - g_list_free(index_keys); } return results; @@ -2848,13 +2847,14 @@ cmd_search_index_all(char* term) auto_gcharv gchar** terms = g_str_tokenize_and_fold(term, NULL, NULL); int terms_len = g_strv_length(terms); - GList* commands = g_hash_table_get_keys(search_index); - GList* curr = commands; - while (curr) { - char* command = curr->data; + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, search_index); + while (g_hash_table_iter_next(&iter, &key, &value)) { + char* command = (char*)key; + char* command_index = (char*)value; int matches = 0; for (int i = 0; i < terms_len; i++) { - char* command_index = g_hash_table_lookup(search_index, command); if (g_str_match_string(terms[i], command_index, FALSE)) { matches++; } @@ -2862,11 +2862,8 @@ cmd_search_index_all(char* term) if (matches == terms_len) { results = g_list_append(results, command); } - curr = g_list_next(curr); } - g_list_free(commands); - return results; } @@ -2989,7 +2986,18 @@ command_docgen(void) } FILE* toc_fragment = fopen("toc_fragment.html", "w"); + if (!toc_fragment) { + log_error("command_docgen(): unable to open toc_fragment.html for writing: %s", g_strerror(errno)); + g_list_free(cmds); + return; + } FILE* main_fragment = fopen("main_fragment.html", "w"); + if (!main_fragment) { + log_error("command_docgen(): unable to open main_fragment.html for writing: %s", g_strerror(errno)); + fclose(toc_fragment); + g_list_free(cmds); + return; + } fputs("