fix(security): prevent CWE-134 format string injection
All checks were successful
CI Code / Check spelling (pull_request) Successful in 21s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Code Coverage (pull_request) Successful in 5m15s
CI Code / Linux (ubuntu) (pull_request) Successful in 6m32s
CI Code / Linux (debian) (pull_request) Successful in 9m14s
CI Code / Linux (arch) (pull_request) Successful in 11m36s
All checks were successful
CI Code / Check spelling (pull_request) Successful in 21s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Code Coverage (pull_request) Successful in 5m15s
CI Code / Linux (ubuntu) (pull_request) Successful in 6m32s
CI Code / Linux (debian) (pull_request) Successful in 9m14s
CI Code / Linux (arch) (pull_request) Successful in 11m36s
- 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
This commit is contained in:
3
.github/workflows/ci-code.yml
vendored
3
.github/workflows/ci-code.yml
vendored
@@ -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
|
||||
|
||||
69
check-cwe134.sh
Executable file
69
check-cwe134.sh
Executable file
@@ -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
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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("");
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user