CWE-121: Stack-based Buffer Overflow - C
Overview
In C, a stack buffer is just a local array or variable with no runtime awareness of its own size once you're past the point where it was declared - char buffer[64] gives the compiler that number at compile time, but nothing stops strcpy(buffer, input) from writing 200 bytes into it at runtime. The classic causes are functions that copy or format data with no destination-size limit (gets, strcpy, sprintf, strcat), and manual loops with an off-by-one or unchecked source length.
Primary Defence: Replace unbounded copy/format functions with size-aware equivalents (fgets, snprintf, and strlcpy/strlcat on BSD, macOS and glibc 2.38 or later), and validate the input length against the destination buffer's actual declared size before copying - never assume the source fits.
Common Vulnerable Patterns
gets() - Always Unsafe
#include <stdio.h>
void read_user_input(void) {
char buffer[64];
// VULNERABLE - gets() has no bounds checking whatsoever
// It reads until newline or EOF regardless of buffer size
gets(buffer); // removed from the C standard library in C11 - never use it
printf("You entered: %s\n", buffer);
}
// Attack: input longer than 64 bytes
// Result: the write runs past buffer into the saved registers and return address
Why this is vulnerable: gets() has no parameter for the destination's size and no way to be told one - it always reads until a newline or end of input, however long that is. There is no safe way to call it, which is why it was removed from the C11 standard entirely.
What the overflow actually achieves depends on the build, and specifically on who built it. A char[64] is inside -fstack-protector-strong's coverage wherever that flag is on, and where it is on differs between the compiler and the distribution's packaging. Ubuntu documents it as the compiler's default protection level, so a plain gcc foo.c there gets a canary. Debian applies it through dpkg-buildflags, and Fedora and RHEL through their hardened GCC specs, which means the distribution's own packages are built with it while a binary a developer compiles by hand on the same machine may not be. Upstream GCC enables it nowhere. Where the canary is present, the corruption is detected at the function's return and the process aborts with *** stack smashing detected *** - still a denial of service and still the bug, but the reachable outcome is a crash rather than code execution. Reaching the return address usefully needs the canary absent (a locally built binary on a distribution that only hardens its packages, an embedded toolchain, an explicit -fno-stack-protector) or a target that sits below it - another array in the same frame, a spilled parameter, a function pointer. Triage the finding on whether the overflow exists, not on whether this particular payload lands a shell, and check how the artefact in front of you was compiled before concluding either way.
strcpy/strcat Without a Size Check
#include <string.h>
void copy_user_data(const char *user_input) {
char buffer[64];
// VULNERABLE - no check that user_input fits in buffer
strcpy(buffer, user_input);
}
void concatenate_strings(const char *str1, const char *str2) {
char result[50];
// VULNERABLE - strcat has no bounds checking either
strcpy(result, str1);
strcat(result, str2); // overflows if str1 + str2 exceed 50 bytes combined
}
Why this is vulnerable: Both functions copy until they hit a NUL terminator in the source, with no awareness of how large the destination actually is. Any combined input longer than the destination's capacity overflows into adjacent stack memory.
sprintf Without Bounds Checking
#include <stdio.h>
void format_message(const char *username, int score) {
char message[50];
// VULNERABLE - sprintf writes as much as the format produces, no size limit
sprintf(message, "User: %s, Score: %d", username, score);
}
Why this is vulnerable: sprintf has no destination-size parameter - it writes exactly as much output as the format string and its arguments produce. A long enough username overflows message regardless of the fixed %d portion.
Off-by-One in a Manual Copy Loop
void copy_with_loop(const char *data) {
char local[10];
// VULNERABLE - loop condition uses <=, writes local[10] which is one past the end
for (int i = 0; i <= 10; i++) {
local[i] = data[i];
}
}
Why this is vulnerable: Valid indices for a 10-byte array are 0 through 9. Using <= instead of < writes one element past the end of local on every call, corrupting whatever the compiler placed immediately after it on the stack.
Secure Patterns
fgets Instead of gets
#include <stdio.h>
#include <string.h>
#define BUFFER_SIZE 64
void read_user_input(void) {
char buffer[BUFFER_SIZE];
// Safe: fgets never writes more than BUFFER_SIZE - 1 bytes plus a NUL terminator
if (fgets(buffer, BUFFER_SIZE, stdin) == NULL) {
fprintf(stderr, "Error reading input\n");
return;
}
size_t len = strlen(buffer);
if (len > 0 && buffer[len - 1] == '\n') {
buffer[len - 1] = '\0';
} else {
// No newline, so the buffer filled up. Either the line ended exactly
// here, or it was longer than the buffer and the rest is still queued.
int c = getchar();
if (c != EOF && c != '\n') {
while (c != '\n' && c != EOF) {
c = getchar(); // discard the remainder of the over-long line
}
fprintf(stderr, "Error: input longer than %d bytes\n", BUFFER_SIZE - 1);
return;
}
}
printf("You entered: %s\n", buffer);
}
Why this works: fgets(buffer, BUFFER_SIZE, stdin) always stops at BUFFER_SIZE - 1 characters, reserving space for the NUL terminator it always adds. Unlike gets(), it requires the caller to state the buffer's size, which is exactly the information gets() had no way to use.
fgets closes the overflow and leaves a second problem behind, which is why the newline check is part of the pattern rather than cosmetic. An over-long line is truncated, not rejected: fgets returns the first BUFFER_SIZE - 1 bytes and leaves the tail in the stream, so the next call reads the middle of the same line as though it were a new one - a parser that takes one command per line then acts on a fragment nobody sent. The absence of a trailing newline is the only signal that this happened, so treat it as an over-long line, drain the stream to the next newline, and reject - matching the reject-rather-than-truncate rule the length-validation pattern below follows. Checking getchar() first distinguishes the case where the line ended exactly at the buffer's capacity or at end of file, which is legitimate input and should not be refused.
Validate Length Before Copying (Recommended)
#include <string.h>
#include <stdio.h>
#define BUFFER_SIZE 64
void copy_user_data(const char *user_input) {
char buffer[BUFFER_SIZE];
size_t input_len = strlen(user_input);
// Validate BEFORE copying - reject rather than silently truncate
if (input_len >= BUFFER_SIZE) {
fprintf(stderr, "Error: input too long (%zu bytes, max %d)\n",
input_len, BUFFER_SIZE - 1);
return;
}
// Safe: validated to fit with room for the NUL terminator
memcpy(buffer, user_input, input_len);
buffer[input_len] = '\0';
}
Why this works: Checking input_len >= BUFFER_SIZE before copying rejects oversized input outright instead of truncating it silently, so the error is visible during testing. memcpy() with a pre-validated length is both safe and predictable - unlike strncpy() (see the pitfall below), it doesn't zero-pad or leave the string unterminated.
Bounded Concatenation Instead of strcat
#include <stdio.h>
#include <string.h>
#define RESULT_SIZE 50
void concatenate_strings(const char *str1, const char *str2) {
char result[RESULT_SIZE];
// Safe: one bounded write produces the whole result, so neither piece can
// overflow and there is no partially-built buffer between the two copies
int written = snprintf(result, sizeof(result), "%s%s", str1, str2);
if (written < 0) {
fprintf(stderr, "Error: formatting failed\n");
return;
}
if ((size_t)written >= sizeof(result)) {
fprintf(stderr, "Error: combined input needs %d characters, max %zu\n",
written, sizeof(result) - 1);
return;
}
printf("Result: %s\n", result);
}
Why this works: snprintf bounds the whole result - both operands and the terminator - against sizeof(result) in a single call, and reports the length it would have needed. The bytes it did write stayed inside result, and the return-value check stops the caller before it treats a truncated value as the real one. strcat cannot do either: it has no size parameter, and it reports nothing. Replacing the pair of calls with one also removes the intermediate state, where result holds str1 and the second copy is still to come.
strlcat(result, str2, sizeof(result)) is the direct replacement where it is available (BSD, macOS, glibc 2.38 or later) and returns the length it would have needed, so truncation is detectable there too. Note the argument it takes is the destination's total size, unlike strncat - see the pitfall below.
snprintf for Formatted Output
#include <stdio.h>
#define MESSAGE_SIZE 50
void format_message(const char *username, int score) {
char message[MESSAGE_SIZE];
// Safe: snprintf enforces the size limit and always NUL-terminates
int written = snprintf(message, MESSAGE_SIZE, "User: %s, Score: %d", username, score);
if (written < 0 || (size_t)written >= MESSAGE_SIZE) {
fprintf(stderr, "Warning: message truncated or encoding error\n");
}
}
Why this works: snprintf() never writes past the size given, and its return value reports how many characters would have been written, so truncation is always detectable rather than silent.
Correct Loop Bounds
void copy_with_loop(const char *data, size_t data_len) {
char local[10];
// Safe: i < sizeof(local) AND i < data_len - both conditions must hold
size_t i;
for (i = 0; i < sizeof(local) - 1 && i < data_len; i++) {
local[i] = data[i];
}
local[i] = '\0';
}
Why this works: i < sizeof(local) - 1 bounds the write to the destination's actual capacity (reserving a byte for the terminator), while i < data_len stops at the source's real length. Neither condition alone is sufficient - the destination bound prevents overflow, and the source bound prevents reading past the end of data.
Testing
- Compile with AddressSanitizer (
-fsanitize=address -fsanitize=undefined -g -O1) and run with normal, boundary (exactly the buffer's capacity), and oversized inputs. - Enable
-fstack-protector-strongand-D_FORTIFY_SOURCE=3and confirm the build still succeeds and canary checks don't trigger under normal use. Level 3 additionally checks buffers whose size is only known at run time, and needs Clang 9+ with glibc 2.33+, or GCC 12+ with glibc 2.35+ - distributions often backport it earlier than that; use=2where the toolchain does not support it. Build these at-O1or higher: glibc'sfeatures.honly activates_FORTIFY_SOURCEwhen__OPTIMIZE__is set, so at-O0it emits#warning _FORTIFY_SOURCE requires compiling with optimization (-O)and compiles nothing extra - a check that has quietly done nothing looks identical to one that passed. - Fuzz any function that copies untrusted input into a fixed-size stack buffer (AFL++, libFuzzer, Honggfuzz).
- Confirm with the sanitizer's output that no out-of-bounds write occurred, not just that the program didn't visibly crash.
Common Pitfalls
- Switching to
strncpy()but sizing it from the source instead of the destination:strncpy(buffer, user_input, strlen(user_input))still copies the full (possibly oversized) source length - the size argument has to come from the destination buffer's capacity (sizeof(buffer) - 1), not the source, or the function name changed without fixing the actual bound. - Forgetting
strncpy()doesn't NUL-terminate on truncation: If the source is exactly as long as (or longer than) the length argument,strncpy()copies that many bytes and does not append a terminator - any later use of the buffer as a C string (printf("%s", buffer)) reads past its end. An explicitbuffer[size - 1] = '\0'after the call is required every time, not just when truncation seems likely. - Passing the destination's size to
strncat():strncat(dest, src, n)appends at mostnbytes and then a terminator, sonis the space remaining after the existing content, not the buffer's capacity -strncat(dest, src, sizeof(dest))can leavestrlen(dest) + sizeof(dest) + 1bytes in a buffer ofsizeof(dest), and is the standard way to overflow while believing the call is bounded. The correct third argument issizeof(dest) - strlen(dest) - 1, which is whysnprintfover both operands orstrlcat(whose third argument is the destination's total size) is the safer replacement forstrcat. - Using
sizeof()on a pointer instead of the array:sizeof(buffer)gives the correct size only whenbufferis the original array in the same scope; once it's passed to another function as a parameter, it decays to a pointer andsizeof(buffer)there returns the pointer's size (4 or 8 bytes), not the array's - the size has to be passed explicitly as a separate parameter. - Relying on
-D_FORTIFY_SOURCEas the fix:_FORTIFY_SOURCEadds runtime checks to several unsafe functions, but how far that reaches depends on the level:=2only helps where the compiler can statically determine the destination's size, and=3extends that to sizes computable at run time but still does not cover every allocation. At either level it's a hardening layer to catch mistakes, not a substitute for validating the length in the code itself.