How buffer overflow happens in C patches.c sprintf macros and how to fix it

Summary

A critical buffer overflow vulnerability was discovered in src/patches.c where the _EPRINT_I, _EPRINT_F, and _EPRINT_COEF macros used sprintf() to write formatted AMY event data into a fixed-size buffer without any bounds checking. By replacing every sprintf() call with snprintf() and tracking remaining buffer space using a s_entry base pointer, the fix ensures that formatting 22 event fields — even at maximum values — can never write beyond the buffer boundary.

Introduction

The src/patches.c file handles serialization of AMY synthesizer events — converting internal event structures into formatted strings for both human-readable debug output and wire protocol transmission. At line 160, a set of three macros — _EPRINT_I, _EPRINT_F, and _EPRINT_COEF — do the heavy lifting of writing each event field into a shared buffer s. The problem: every single one of those writes used sprintf(), which has no concept of how much space remains in the buffer.

This is a textbook example of a vulnerability that looks harmless in isolation but becomes critical when you count the fields. AMY events have 22 printable fields: 12 integers (freq, midi_note, velocity, patch, wave, osc, time, channel, bp0–bp3) and 10 floats (amp, phase, feedback, filter_freq, filter_type, resonance, pan, pitch_bend, ratio, detune), plus coefficient arrays. With all fields set to maximum values, the cumulative output easily overflows any reasonably sized stack buffer.

The Vulnerability Explained

The Vulnerable Macro Pattern

Here is the exact code that was vulnerable, at src/patches.c:160:

// BEFORE — vulnerable
#define _EPRINT_I(FIELD, NAME, WIRECODE) \
    if (AMY_IS_SET(e->FIELD)) { \
        sprintf(s, "%s%" PRId32, wirecode ? WIRECODE : " " NAME ": ", (int32_t)e->FIELD); \
        s += strlen(s); \
    }

#define _EPRINT_F(FIELD, NAME, WIRECODE) \
    if (AMY_IS_SET(e->FIELD)) { \
        sprintf(s, "%s%.3f", wirecode ? WIRECODE : " " NAME ": ", e->FIELD); \
        s += strlen(s); \
    }

And the coefficient array macro:

// BEFORE — vulnerable
#define _EPRINT_COEF(FIELD, NAME, WIRECODE) {
    ...
    if (last_set >= 0) {
        sprintf(s, "%s", wirecode ? WIRECODE : " " NAME ": ");
        s += strlen(s);
        for (int i = 0; i <= last_set; ++i) {
            if (i > 0) { sprintf(s, ","); s += strlen(s); }
            if (AMY_IS_SET(e->FIELD[i])) {
                sprintf(s, "%.3f", e->FIELD[i]);
                s += strlen(s);
            }
        }
    }
}

Why This Is Dangerous

The critical flaw is the combination of two patterns:

1. sprintf() writes without a size limit — it will write as many bytes as the formatted output requires, regardless of how much buffer space remains.
2. s += strlen(s) advances the pointer — after each write, the pointer moves forward to the end of what was just written. There is no check that s is still within the original buffer's bounds.

Each macro call is harmless in isolation when only one or two fields are set. But when all 22 fields are populated — which is exactly what happens with a fully specified AMY event — the writes accumulate. Consider the worst case:

• Each integer field like freq: 2147483647 contributes ~20 bytes
• Each float field like filter_freq: 340282346638528860000000000000000000000.000 contributes up to ~50 bytes
• 12 integer fields × 20 bytes = ~240 bytes
• 10 float fields × 50 bytes = ~500 bytes
• Total: ~740+ bytes, easily overflowing a 512-byte or smaller stack buffer

The Attack Scenario

An attacker controlling AMY wire protocol input can craft a message with all possible fields set to maximum values (INT32_MAX for integers, FLT_MAX for floats). When the application processes this message and calls the event-printing function, each _EPRINT_I and _EPRINT_F macro fires in sequence, advancing s past the end of the buffer. The bytes written beyond the buffer boundary overwrite adjacent stack memory — potentially including saved return addresses, function pointers, or other security-critical data.

This is not theoretical. The regression test included with the fix explicitly demonstrates that unsafe_format_event() (the old behavior) overflows a 64-byte buffer when all fields are set to 12345678 and 9876.543f, while safe_format_event() (the new behavior) correctly returns an error and leaves canary bytes intact.

The Fix

Replacing sprintf() with snprintf() Throughout

The fix makes a surgical but comprehensive change: every sprintf() call in the three macros is replaced with snprintf(), and each call receives the remaining buffer capacity as its size argument. This is computed using a base pointer s_entry that records where the buffer starts, allowing the expression len - (size_t)(s - s_entry) to give the exact number of bytes still available at each point.

// AFTER — safe
#define _EPRINT_I(FIELD, NAME, WIRECODE) \
    if (AMY_IS_SET(e->FIELD)) { \
        snprintf(s, len - (size_t)(s - s_entry), \
                 "%s%" PRId32, wirecode ? WIRECODE : " " NAME ": ", (int32_t)e->FIELD); \
        s += strlen(s); \
    }

#define _EPRINT_F(FIELD, NAME, WIRECODE) \
    if (AMY_IS_SET(e->FIELD)) { \
        snprintf(s, len - (size_t)(s - s_entry), \
                 "%s%.3f", wirecode ? WIRECODE : " " NAME ": ", e->FIELD); \
        s += strlen(s); \
    }

And for the coefficient array:

// AFTER — safe
#define _EPRINT_COEF(FIELD, NAME, WIRECODE) {
    ...
    if (last_set >= 0) {
        snprintf(s, len - (size_t)(s - s_entry), "%s", wirecode ? WIRECODE : " " NAME ": ");
        s += strlen(s);
        for (int i = 0; i <= last_set; ++i) {
            if (i > 0) {
                snprintf(s, len - (size_t)(s - s_entry), ",");
                s += strlen(s);
            }
            if (AMY_IS_SET(e->FIELD[i])) {
                snprintf(s, len - (size_t)(s - s_entry), "%.3f", e->FIELD[i]);
                s += strlen(s);
            }
        }
    }
}

Before vs. After Comparison

Why len - (size_t)(s - s_entry) Is the Right Pattern

The key insight is that s - s_entry gives the number of bytes already written since the start of the buffer. Subtracting that from len (the total buffer size) gives the exact remaining capacity. This is computed fresh on every macro invocation, which is essential — a single check at the top of the function would not account for the space consumed by earlier fields.

This pattern is also safe from integer underflow: since s is only ever advanced forward by strlen(s) (which is non-negative), s - s_entry is always ≤ len as long as snprintf is doing its job, which it is.

Prevention & Best Practices

Never Use sprintf() in Accumulating-Pointer Patterns

Any time you see code of the form:

char buf[N];
char *s = buf;
sprintf(s, ...);
s += strlen(s);
sprintf(s, ...);   // ← danger: no remaining-space check
s += strlen(s);

treat it as a buffer overflow waiting to happen. The fix is always the same: track remaining capacity and use snprintf().

The Canonical Safe Pattern in C

char buf[N];
char *s = buf;
const char *s_entry = buf;  // base pointer, never moves
size_t remaining;

remaining = N - (size_t)(s - s_entry);
int written = snprintf(s, remaining, "field: %d", value);
if (written > 0 && (size_t)written < remaining) {
    s += written;
}

Note that using snprintf's return value directly (rather than strlen(s) after the call) is slightly more efficient and avoids a redundant string scan.

Use Compiler and Sanitizer Warnings

• -D_FORTIFY_SOURCE=2 on GCC/Clang: enables runtime checks for certain format-string buffer overflows.
• AddressSanitizer (-fsanitize=address): will catch out-of-bounds writes immediately in testing.
• -Wformat-overflow (GCC 7+): warns at compile time when sprintf output may overflow a known-size buffer.
• Static analysis: Semgrep rule c.lang.security.insecure-use-sprintf-fn and cppcheck --enable=all both flag unbounded sprintf() calls.

Relevant Standards

• CWE-121: Stack-based Buffer Overflow — the direct classification for this vulnerability
• CWE-787: Out-of-bounds Write — the broader category
• CWE-676: Use of Potentially Dangerous Function — covers sprintf specifically
• OWASP: Buffer Overflow Prevention Cheat Sheet
• SEI CERT C: Rule STR07-C — "Use the bounds-checking interfaces for string manipulation"

Key Takeaways

• The _EPRINT_I and _EPRINT_F macros in patches.c are called up to 22 times in sequence — any unbounded write function in a macro that runs this many times is a buffer overflow candidate, even if each individual write looks small.
• s += strlen(s) after sprintf() is a red flag pattern — it means the code is accumulating writes with pointer arithmetic but has no mechanism to stop when the buffer is full.
• snprintf() alone is not sufficient — you must pass the correct remaining size — passing a stale or incorrect size (e.g., always passing N instead of N - bytes_written) defeats the protection.
• The _EPRINT_COEF loop needed fixing too — the inner for loop over coefficient array entries had the same sprintf() pattern, requiring snprintf with remaining-space tracking on every iteration, not just the first.
• AMY wire protocol input is an attacker-controlled source — any parsing or formatting code that operates on wire data must be hardened against adversarial maximum-value inputs across all fields simultaneously.

How Orbis AppSec Detected This

• Source: Crafted AMY wire protocol messages with all event fields set to maximum values (INT32_MAX for integers, FLT_MAX for floats), entering the event-formatting code path in src/patches.c.
• Sink: sprintf(s, "%s%" PRId32, ...) and sprintf(s, "%s%.3f", ...) inside the _EPRINT_I and _EPRINT_F macros at src/patches.c:160, called cumulatively for up to 22 fields with no bounds checking.
• Missing control: No size argument was passed to sprintf(), and no check was made on remaining buffer capacity before or after each macro invocation; the pointer s was advanced without limit.
• CWE: CWE-121 — Stack-based Buffer Overflow (also CWE-787: Out-of-bounds Write; CWE-676: Use of Potentially Dangerous Function).
• Fix: Replaced all sprintf() calls with snprintf(s, len - (size_t)(s - s_entry), ...), introducing a base pointer s_entry to compute remaining capacity on every write.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

The buffer overflow in src/patches.c is a clear illustration of how safe-looking code becomes dangerous at scale. Each individual sprintf() write in _EPRINT_I or _EPRINT_F looks benign — it's just formatting one integer or float. But macros are called in sequence for every set field in an event, and with 22 possible fields all active, the cumulative output blows past any fixed buffer. The fix is precise and minimal: snprintf() with a dynamically computed remaining-size argument, anchored by a s_entry base pointer that never moves.

If you write C code that serializes structured data into fixed-size buffers using pointer-advancing loops or macros, audit every sprintf() call. The pattern sprintf(s, ...); s += strlen(s); repeated N times is a buffer overflow in proportion to N. Replace it with snprintf(s, remaining, ...); s += written; remaining -= written; and your buffer will never overflow regardless of how many fields are set or how large their values are.

References

• CWE-121: Stack-based Buffer Overflow
• CWE-787: Out-of-bounds Write
• CWE-676: Use of Potentially Dangerous Function
• OWASP C-Based Toolchain Hardening Cheat Sheet
• snprintf() — C Standard Library Reference (cppreference)
• Semgrep rule: insecure use of sprintf
• fix: use snprintf in patches.c