How strcpy buffer overflow happens in C++ debugger command handling and how to fix it

How strcpy Buffer Overflow Happens in C++ Debugger Command Handling and How to Fix It

Introduction

In src/debugger.cpp at line 387, a critical stack-based buffer overflow was discovered in the Debugger::handle_command() function. The vulnerability existed because strcpy was used to copy user-entered debugger command strings directly into prevCommandBuffer — a fixed-size stack-allocated character array — without any bounds checking whatsoever.

This is a game/emulator debugger, meaning exploitation could be triggered by loading a crafted ROM, save file, or game asset that feeds oversized strings into the debugger's command processing pipeline. The result? An attacker could overwrite the stack's return address and achieve arbitrary code execution on the host system.

The Vulnerability Explained

The vulnerable code in Debugger::handle_command() looked like this:

strcpy(prevCommandBuffer, command.c_str());
strcpy(commandBuffer, "");

Here's why this is dangerous:

1. prevCommandBuffer is a fixed-size stack buffer — likely 256 or 512 bytes based on typical debugger implementations.
2. command is a std::string derived from user input with no inherent size limit.
3. strcpy copies until it hits a null terminator — it has absolutely no concept of the destination buffer's capacity.

When a user (or a crafted input source) provides a command string longer than prevCommandBuffer's allocated size, strcpy happily writes past the end of the buffer and into adjacent stack memory.

Attack Scenario

Consider this exploitation path specific to this emulator:

1. An attacker crafts a ROM or save file that, when loaded, triggers the debugger (e.g., via a breakpoint or debug trap instruction).
2. The crafted input feeds a command string of 300+ bytes into handle_command().
3. strcpy(prevCommandBuffer, command.c_str()) writes all 300+ bytes into a 256-byte buffer.
4. The 44+ overflow bytes overwrite the saved frame pointer and return address on the stack.
5. When handle_command() returns, execution jumps to the attacker-controlled address.
6. The attacker now has arbitrary code execution with the privileges of the emulator process.

The second strcpy(commandBuffer, "") is also problematic — while it only copies a single null byte, using strcpy to clear a buffer is an anti-pattern that obscures intent and could mask issues if the code evolves.

The Fix

The fix replaces both unbounded strcpy calls with safe, bounded alternatives:

Before (vulnerable):

strcpy(prevCommandBuffer, command.c_str());
strcpy(commandBuffer, "");

After (fixed):

strncpy(prevCommandBuffer, command.c_str(), sizeof(prevCommandBuffer) - 1);
prevCommandBuffer[sizeof(prevCommandBuffer) - 1] = '\0';
commandBuffer[0] = '\0';

Here's what each line accomplishes:

1. strncpy(prevCommandBuffer, command.c_str(), sizeof(prevCommandBuffer) - 1) — Copies at most sizeof(prevCommandBuffer) - 1 bytes from the command string. Even if command is 10,000 characters long, only the buffer's capacity minus one byte will be written.

2. prevCommandBuffer[sizeof(prevCommandBuffer) - 1] = '\0' — Explicitly null-terminates the buffer. This is critical because strncpy does NOT null-terminate when the source is longer than the specified count. Without this line, prevCommandBuffer could be a non-terminated string, leading to subsequent reads going out of bounds.

3. commandBuffer[0] = '\0' — Replaces the unnecessary strcpy(commandBuffer, "") with a direct null-byte assignment. This is cleaner, faster, and communicates intent: "empty this buffer."

The fix ensures that regardless of input length, the write to prevCommandBuffer never exceeds its allocated size, and the buffer is always properly null-terminated.

Prevention & Best Practices

Immediate Actions

• Ban strcpy in your codebase. Use compiler flags (-Werror=deprecated-declarations on some platforms) or linting rules to flag any use of strcpy.
• Prefer std::string over raw char[] buffers in C++. The original command variable was already a std::string — prevCommandBuffer should ideally be one too.
• If you must use C-style strings, always use strncpy + explicit null-termination, or better yet, snprintf(dest, sizeof(dest), "%s", src) which always null-terminates.

Compiler & Runtime Protections

• Enable stack canaries (-fstack-protector-strong) to detect stack smashing at runtime.
• Enable ASLR and DEP/NX to make exploitation harder even if an overflow occurs.
• Use AddressSanitizer (-fsanitize=address) during development to catch overflows immediately.

Static Analysis

• Tools like Semgrep, Coverity, and Clang's static analyzer can flag strcpy with user-controlled sources.
• GCC's -Wstringop-overflow can catch some cases at compile time.

Relevant Standards

• CWE-121: Stack-based Buffer Overflow
• CWE-120: Buffer Copy without Checking Size of Input
• OWASP: Memory Safety guidelines

Key Takeaways

• Never use strcpy() with any input that could exceed the destination buffer — in Debugger::handle_command(), the command string has no size guarantee, making strcpy into prevCommandBuffer a ticking time bomb.
• strncpy alone is NOT safe — you must always explicitly null-terminate with dest[size-1] = '\0' because strncpy silently drops the terminator when truncating.
• Clearing a buffer with strcpy(buf, "") is an anti-pattern — use buf[0] = '\0' for clarity and safety.
• Game/emulator debuggers are attack surfaces — crafted ROMs or save files can trigger debugger code paths, making seemingly "developer-only" code exploitable in production.
• sizeof(prevCommandBuffer) is the correct bound — using sizeof on the actual destination array ensures the limit stays correct even if the buffer size changes in the future.

How Orbis AppSec Detected This

• Source: User-entered debugger command string processed by Debugger::handle_command(char* commandBuffer) — input arrives via the debugger's command prompt or potentially through crafted game assets that trigger debug traps.
• Sink: strcpy(prevCommandBuffer, command.c_str()) at src/debugger.cpp:387 — an unbounded copy into a fixed-size stack buffer.
• Missing control: No length validation or bounded copy operation between the variable-length command string and the fixed-size prevCommandBuffer.
• CWE: CWE-121 (Stack-based Buffer Overflow)
• Fix: Replaced strcpy with strncpy bounded by sizeof(prevCommandBuffer) - 1 with explicit null-termination, and replaced strcpy(commandBuffer, "") with direct null-byte assignment.

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

This vulnerability demonstrates a classic but still-prevalent pattern: using strcpy to copy variable-length user input into a fixed-size buffer. In the context of a game emulator's debugger, this isn't just a theoretical concern — crafted ROMs and save files can trigger debug code paths, turning a seemingly benign developer tool into an arbitrary code execution vector.

The fix is minimal but effective: three lines that enforce a hard upper bound on the copy operation and guarantee null-termination. If you're working in C or C++ with character buffers, audit every strcpy call in your codebase today. Replace them with bounded alternatives, and consider whether std::string would eliminate the risk entirely.

References

• CWE-121: Stack-based Buffer Overflow
• CWE-120: Buffer Copy without Checking Size of Input
• OWASP Buffer Overflow Prevention
• strncpy documentation — cppreference.com
• Semgrep rules for buffer overflow
• fix: use bounded strlcpy/snprintf in debugger.cpp