How buffer overflow happens in C gdb-server and how to fix it

How Buffer Overflow Happens in C gdb-server and How to Fix It

Introduction

In the st-util GDB server component (src/st-util/gdb-server.c), a critical buffer overflow vulnerability was discovered that could allow an attacker to corrupt heap memory by supplying a maliciously long --serial command-line argument. The vulnerability stems from four locations in the code where unbounded string operations—strcpy() and memcpy()—copy data into fixed-size buffers without checking if the source fits. Specifically:

• Line 212: memcpy(st->serialnumber, optarg, STLINK_SERIAL_BUFFER_SIZE) incorrectly assumes optarg is smaller than the buffer size
• Line 367 & 376: Two strcpy() calls copy template strings into a dynamically allocated map buffer without size validation
• Line 994: strncpy() is used but lacks null-termination guarantee

This is a classic memory safety issue in C that affects any developer working with command-line tools, embedded systems, or low-level utilities. Understanding this vulnerability and its fix is essential for writing secure C code.

The Vulnerability Explained

The Problematic Code

Let's examine the vulnerable code at line 212 in the parse_options() function:

case SERIAL_OPTION:
    printf("use serial %s\n", optarg);
    memcpy(st->serialnumber, optarg, STLINK_SERIAL_BUFFER_SIZE);
    break;

The issue is subtle but dangerous: the code uses memcpy() with STLINK_SERIAL_BUFFER_SIZE as the copy length, but STLINK_SERIAL_BUFFER_SIZE defines the destination buffer size, not the source length. The optarg pointer comes directly from the command-line argument parser and has no length validation.

If an attacker runs:

./st-util --serial AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

with a serial string longer than STLINK_SERIAL_BUFFER_SIZE (typically 16 bytes), the memcpy() call will write beyond the st->serialnumber buffer boundary, corrupting adjacent heap memory.

Similar Issues in Memory Map Functions

The same pattern appears in the make_memory_map() function at lines 367 and 376:

} else if(sl->chip_id == STM32_CHIPID_F4_DE) {
    strcpy(map, memory_map_template_F4_DE);  // Line 367 - DANGEROUS!
} else if(sl->chip_id == STM32_CHIPID_F4_HD) {
    strcpy(map, memory_map_template_F4_HD);  // Line 376 - DANGEROUS!

Here, strcpy() copies a template string into the map buffer. While map is dynamically allocated with calloc(), the allocation size is computed earlier in the function. If the template string is larger than the allocated size (a possible scenario in firmware updates or configuration changes), the strcpy() will overflow.

Attack Scenario

An attacker who can control the command-line arguments to st-util can trigger this vulnerability:

# Craft a serial number exceeding STLINK_SERIAL_BUFFER_SIZE
./st-util --serial "$(python3 -c 'print("A" * 100)')"

The memcpy() call writes 16 bytes from a 100-byte string, but the destination buffer is only 16 bytes. The extra 84 bytes corrupt:
- Adjacent heap metadata
- Other heap-allocated structures
- Potentially enabling a heap exploit or denial of service

Real-World Impact

For a tool like st-util (used for STM32 microcontroller debugging), this vulnerability could:
- Crash the debugger during firmware upload/download operations
- Corrupt debug session state, making debugging unreliable
- Escalate privileges if st-util runs with elevated permissions (less common but possible in embedded development environments)
- Enable remote code execution if the heap corruption is leveraged with a heap spray attack

The Fix

What Changed

The fix replaces all four unsafe string operations with bounds-checked alternatives:

Fix 1: Line 212 - Replace memcpy with snprintf

Before:

memcpy(st->serialnumber, optarg, STLINK_SERIAL_BUFFER_SIZE);

After:

snprintf(st->serialnumber, STLINK_SERIAL_BUFFER_SIZE, "%s", optarg);

Why this works: snprintf() enforces the buffer size limit (second parameter) and automatically null-terminates the string. Even if optarg is 1000 bytes, snprintf() will copy at most STLINK_SERIAL_BUFFER_SIZE - 1 bytes and add a null terminator. The string is safely truncated, not overflowed.

Fix 2 & 3: Lines 367 & 376 - Replace strcpy with snprintf

Before (Line 367):

strcpy(map, memory_map_template_F4_DE);

After:

snprintf(map, sz, "%s", memory_map_template_F4_DE);

Why this works: The variable sz holds the allocated size of map. By passing sz to snprintf(), we ensure the template string cannot exceed the buffer boundary. If the template is larger than sz, it's truncated safely.

Fix 4: Line 994 - Replace strncpy with memcpy + explicit size

Before:

strncpy(queryName, &packet[1], queryNameLength);

After:

memcpy(queryName, &packet[1], queryNameLength);

Why this works: The original code used strncpy(), which doesn't guarantee null-termination if the source is exactly queryNameLength bytes. The buffer queryName is allocated with calloc(1, queryNameLength + 1), which zeros memory. Using memcpy() with the exact size and relying on the zero-initialized buffer is safe and more efficient.

Security Improvement

The fixes enforce a security invariant: no string operation will write beyond its destination buffer boundary, even with adversarial input.

Key Takeaways

• Never copy user-controlled data with unbounded functions. The --serial argument in line 212 is attacker-controlled; memcpy() with no length check is a direct buffer overflow.

• strcpy() is unsafe in all contexts. Even if you "know" the string is small, future code changes or configuration updates can introduce longer strings. Use snprintf() instead.

• snprintf() is your friend. It enforces buffer boundaries, null-terminates automatically, and safely truncates. It's the standard way to format and copy strings in modern C.

• Template strings must be validated against buffer size. The memory map templates in lines 367 and 376 are "internal" strings, but they can still overflow if the buffer size calculation is wrong. Always use bounded functions.

• Compiler protections catch what code review misses. Stack canaries and AddressSanitizer would have detected this vulnerability during testing. Use them in development and CI/CD.

How Orbis AppSec Detected This

Source: Command-line argument optarg from getopt() parser, passed directly to memcpy() without length validation.

Sink:
- memcpy(st->serialnumber, optarg, STLINK_SERIAL_BUFFER_SIZE) at line 212
- strcpy(map, memory_map_template_F4_DE) at line 367
- strcpy(map, memory_map_template_F4_HD) at line 376
- strncpy(queryName, &packet[1], queryNameLength) at line 994

Missing Control: No validation that source data length fits within destination buffer size. No use of bounded string functions.

CWE: CWE-120 – Buffer Copy without Checking Size of Input

Fix: Replaced strcpy() and memcpy() with snprintf() (which enforces the buffer size parameter) and corrected strncpy() to use memcpy() with explicit size validation from buffer allocation.

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

Buffer overflow vulnerabilities in C are among the most critical security issues, with a history spanning decades of exploits and breaches. The fix in st-util demonstrates a fundamental principle: use bounded functions by default, never unbounded ones.

The shift from strcpy() to snprintf(), from memcpy() without checks to memcpy() with validated sizes, and from strncpy() without null-termination guarantees to safer alternatives represents a maturation of secure C coding practices.

If you maintain C code, especially low-level tools like debuggers, embedded systems, or security-critical applications:

1. Audit your codebase for strcpy(), sprintf(), and unbounded memcpy() calls
2. Replace them with snprintf(), strlcpy(), or bounded alternatives
3. Enable compiler protections during development and testing
4. Use static analysis in your CI/CD pipeline to catch new issues before they reach production

The developers of st-util made the right choice in fixing this issue proactively. Your code can be just as secure.