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

Make security audit for CWE-134 #85

New Issue
Closed
opened 2026-02-05 11:56:07 +00:00 by jabber.developer · 1 comment
jabber.developer commented 2026-02-05 11:56:07 +00:00
Owner
Copy Link

CWE-134, per mitre, is a vulnerability that happens when the product uses a function that accepts a format string as an argument, but the format string originates from an external source.

Currently, cons_show does not always securely display values. For example, the following line could be exploited:

src/ui/console.c Line 941 in f8826b7c79
cons_show(accounts[i]);

The following code:

/account add "%p%p%p"
/account list
/account remove "%p%p%p"

Will output list of accounts, one of which will display values from the stack instead of "%p%p%p" due to formatting string injection.

Full audit will include multiple steps:

  1. Identify all methods that can be used in template injection attack:
    • cons_show
    • printf
  2. Search for all of them via regex; identify insecure usage

To prevent reintroduction of these vulnerabilities in the future, we will include these regexes as part of the CI/CD.

CWE-134, per [mitre](https://cwe.mitre.org/data/definitions/134.html), is a vulnerability that happens when the product uses a function that accepts a format string as an argument, but the format string originates from an external source. Currently, `cons_show` does not always securely display values. For example, the following line could be exploited: https://git.jabber.space/devs/cproof/src/commit/f8826b7c79c37c06a0dd9790cbdbb7a4500b480e/src/ui/console.c#L941 The following code: ```bash /account add "%p%p%p" /account list /account remove "%p%p%p" ``` Will output list of accounts, one of which will display values from the stack instead of "%p%p%p" due to formatting string injection. Full audit will include multiple steps: 1. Identify all methods that can be used in template injection attack: - `cons_show` - `printf` 2. Search for all of them via regex; identify insecure usage To prevent reintroduction of these vulnerabilities in the future, we will include these regexes as part of the CI/CD.
jabber.developer added the
Kind/Security
Priority
Critical
1
Reviewed
Confirmed
1
 labels 2026-02-05 11:56:44 +00:00
jabber.developer added this to the Plans (2025-2026) project 2026-02-05 11:56:47 +00:00
jabber.developer moved this to In Progress in Plans (2025-2026) on 2026-02-05 11:56:59 +00:00
jabber.developer moved this to Verification in Plans (2025-2026) on 2026-02-06 19:02:14 +00:00
jabber.developer2 commented 2026-02-26 16:48:53 +00:00
Collaborator
Copy Link

Description

g_strdup_vprintf(message, arg) in window.c is a format-string sink that receives its format argument indirectly through wrapper functions like win_println_va_internal, win_print, win_println, etc. If any caller up the chain passes a user-controlled string as the message parameter without it being a format literal (or without wrapping it in "%s"), this constitutes a CWE-134: Use of Externally-Controlled Format String vulnerability.

Problem

The current grep-based check (check-cwe134.sh) can only detect direct misuse at the immediate call site (e.g., cons_show(variable);). It cannot detect tainted data flowing through multiple function calls before reaching the printf-family sink. For example:

// In some_module.c — called with user-controlled 'text'
void show_incoming(const char* text) {
    some_helper(window, text);  // passes through
}

// In helper.c
void some_helper(ProfWin* win, const char* msg) {
    win_println(win, THEME_DEFAULT, "-", msg);  // CWE-134! 'msg' used as format string
}

// In window.c — ultimately reaches:
gchar* msg = g_strdup_vprintf(message, arg);  // format string sink

A %n or other format specifier in the user-controlled string could cause a crash or arbitrary write.

Suggested Fix

  1. Add GCC format attributes to all printf-like wrapper functions in window.c and window.h so the compiler can enforce that callers pass format literals:

    // For va_list variants (can't check caller args, but marks the format param):
__attribute__((format(printf, 4, 0)))
void win_println_va(ProfWin* window, theme_item_t theme_item,
                    const char* show_char, const char* const message, va_list arg);

// For variadic variants (compiler checks caller args against format):
__attribute__((format(printf, 4, 5)))
void win_println(ProfWin* window, theme_item_t theme_item,
                 const char* show_char, const char* const message, ...);

  2. Enable -Wformat-nonliteral in CFLAGS to flag any call site that passes a non-literal as the format argument to an attributed function.

  3. Add -fanalyzer to CI (GCC 11+) which performs interprocedural taint analysis and will emit -Wanalyzer-tainted-format-string when externally-sourced data reaches a format sink across arbitrary call depth.

  4. Consider CodeQL (cpp/tainted-format-string query) for deeper cross-translation-unit taint tracking, especially if the project is hosted on GitHub (free for public repos).

### Description `g_strdup_vprintf(message, arg)` in window.c is a format-string sink that receives its format argument indirectly through wrapper functions like `win_println_va_internal`, `win_print`, `win_println`, etc. If any caller up the chain passes a user-controlled string as the `message` parameter without it being a format literal (or without wrapping it in `"%s"`), this constitutes a **CWE-134: Use of Externally-Controlled Format String** vulnerability. ### Problem The current grep-based check (check-cwe134.sh) can only detect direct misuse at the immediate call site (e.g., `cons_show(variable);`). It **cannot** detect tainted data flowing through multiple function calls before reaching the `printf`-family sink. For example: ```c // In some_module.c — called with user-controlled 'text' void show_incoming(const char* text) { some_helper(window, text); // passes through } // In helper.c void some_helper(ProfWin* win, const char* msg) { win_println(win, THEME_DEFAULT, "-", msg); // CWE-134! 'msg' used as format string } // In window.c — ultimately reaches: gchar* msg = g_strdup_vprintf(message, arg); // format string sink ``` A `%n` or other format specifier in the user-controlled string could cause a crash or arbitrary write. ### Suggested Fix 1. **Add GCC `format` attributes** to all `printf`-like wrapper functions in window.c and window.h so the compiler can enforce that callers pass format literals: ```c // For va_list variants (can't check caller args, but marks the format param): __attribute__((format(printf, 4, 0))) void win_println_va(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, va_list arg); // For variadic variants (compiler checks caller args against format): __attribute__((format(printf, 4, 5))) void win_println(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, ...); ``` 2. **Enable `-Wformat-nonliteral`** in `CFLAGS` to flag any call site that passes a non-literal as the format argument to an attributed function. 3. **Add `-fanalyzer` to CI** (GCC 11+) which performs interprocedural taint analysis and will emit `-Wanalyzer-tainted-format-string` when externally-sourced data reaches a format sink across arbitrary call depth. 4. **Consider CodeQL** (`cpp/tainted-format-string` query) for deeper cross-translation-unit taint tracking, especially if the project is hosted on GitHub (free for public repos).
jabber.developer moved this to Done in Plans (2025-2026) on 2026-04-18 16:48:08 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Kind/Security
Priority
Critical
1
Reviewed
Confirmed
1
Milestone
No items
No Milestone
Projects
Clear projects
No project Plans (2025-2026)
Assignees
Clear assignees
admin jabber.developer jabber.developer2
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: devs/cproof#85
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?