How buffer overflow in strcpy() happens in C configuration parsing and how to fix it

Introduction

In the rpconfig.h file of this C codebase, we discovered a critical buffer overflow vulnerability at line 482 within the rpcSetText() function. This function handles configuration text entries, copying user-provided values into fixed-size buffers using the notoriously unsafe strcpy() function. Without any bounds checking, an attacker could supply a malicious configuration file with text values exceeding the 256-byte buffer limit, triggering a stack-based buffer overflow that corrupts adjacent memory and potentially enables arbitrary code execution.

This vulnerability exemplifies why strcpy() has been considered dangerous for decades—it blindly copies data until it encounters a null terminator, regardless of destination buffer size. The rpcSetText() function processes configuration keys and their associated text values, making it a prime target for exploitation through crafted configuration files. With the buffer allocated on the stack and no length validation, this represents a textbook case of CWE-120: Buffer Copy without Checking Size of Input.

The Vulnerability Explained

The vulnerable code in src/rpconfig.h at line 482 looks like this:

int rpcSetText(rpcProjectConfig config, const char *key, const char *text)
{
    // ... earlier code ...

    if (TextIsEqual(config.entries[i].key, key) &&
        !TextIsEqual(config.entries[i].text, text))
    {
        memset(config.entries[i].text, 0, 256);
        strcpy(config.entries[i].text, text);  // VULNERABLE LINE
        result = i;
        break;
    }
}

The problem is crystal clear: config.entries[i].text is a fixed 256-byte buffer (as evidenced by the memset() call on the previous line), but strcpy() copies the entire text parameter without checking its length. If text contains more than 255 characters (256 minus the null terminator), strcpy() will write beyond the buffer boundary, overwriting whatever memory comes next on the stack.

How the Exploitation Works:

An attacker could craft a malicious configuration file with a text value exceeding 256 bytes:

[Section]
key1 = AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA...
       [continues for 300+ characters]

When rpcSetText() processes this entry, here's what happens:

1. The function finds the matching key in config.entries[i]
2. It clears the 256-byte text buffer with memset()
3. It calls strcpy() to copy the 300+ character string
4. strcpy() writes 256 bytes into the buffer, then continues writing the remaining 44+ bytes into adjacent stack memory
5. This overwrites local variables, saved frame pointers, or even the return address
6. If the attacker controls the overwritten data precisely, they can redirect program execution

Real-World Impact:

This vulnerability affects production code that parses configuration files. The consequences include:

• Memory corruption: Overwriting adjacent stack variables leads to unpredictable program behavior and crashes
• Information disclosure: Corrupted pointers might leak sensitive memory contents
• Arbitrary code execution: By carefully crafting the overflow payload, an attacker could overwrite the return address to hijack control flow
• Privilege escalation: If this code runs with elevated privileges, successful exploitation could grant system-level access

The PR notes that line 515 in the same file uses a similar pattern, suggesting this vulnerability might exist in multiple locations within the configuration parsing logic.

The Fix

The fix is elegant and effective—replacing the unsafe strcpy() with strncpy() and enforcing a maximum copy length:

Before (Vulnerable):

memset(config.entries[i].text, 0, 256);
strcpy(config.entries[i].text, text);

After (Secure):

memset(config.entries[i].text, 0, 256);
strncpy(config.entries[i].text, text, 255);

Why This Works:

The change from strcpy() to strncpy() with a limit of 255 bytes provides multiple layers of protection:

1. Bounds enforcement: strncpy() will never write more than 255 bytes into the destination buffer, preventing overflow
2. Null terminator space: Limiting to 255 bytes leaves room for the null terminator in the 256-byte buffer
3. Predictable behavior: Even with malicious input exceeding 255 characters, the function safely truncates the string

The fix specifically uses 255 (not 256) as the length parameter because:
- The buffer is 256 bytes total
- We need to reserve one byte for the null terminator
- strncpy() doesn't automatically add a null terminator if the source is longer than the limit
- The prior memset() call ensures the buffer is zero-initialized, providing null termination

Important Note on strncpy() Behavior:

While this fix is secure, developers should understand that strncpy() has quirky behavior:
- If the source string is shorter than the limit, strncpy() pads the remaining space with null bytes
- If the source string is longer than the limit, strncpy() does NOT add a null terminator
- In this case, the memset() call beforehand ensures null termination, making the fix safe

Additional Protection:

The PR also includes a comprehensive regression test that validates the buffer length invariant with various payload sizes, including boundary conditions and extreme overflow attempts. This test ensures future code changes won't reintroduce the vulnerability.

Prevention & Best Practices

1. Avoid Unsafe String Functions

Never use these functions in production code:
- strcpy() → Use strncpy(), strlcpy(), or snprintf()
- strcat() → Use strncat() or strlcat()
- sprintf() → Use snprintf()
- gets() → Use fgets()

2. Always Validate Input Length

Before copying any user-controlled data:

if (strlen(text) >= sizeof(config.entries[i].text)) {
    // Handle error: input too long
    return ERROR_INPUT_TOO_LONG;
}
strncpy(config.entries[i].text, text, sizeof(config.entries[i].text) - 1);
config.entries[i].text[sizeof(config.entries[i].text) - 1] = '\0';

3. Use Safer Alternatives

Consider modern C string handling approaches:

// Option 1: snprintf() for guaranteed null termination
snprintf(config.entries[i].text, sizeof(config.entries[i].text), "%s", text);

// Option 2: strlcpy() if available (BSD systems)
strlcpy(config.entries[i].text, text, sizeof(config.entries[i].text));

4. Enable Compiler Protections

Modern compilers offer runtime protections:
- Stack canaries (-fstack-protector-strong)
- Address Space Layout Randomization (ASLR)
- Data Execution Prevention (DEP/NX)
- Fortify Source (-D_FORTIFY_SOURCE=2)

5. Static Analysis Integration

Integrate tools that detect unsafe string operations:
- Semgrep: Rules like dangerous-strcpy flag these patterns
- Clang Static Analyzer: Detects buffer overflows at compile time
- Coverity: Commercial tool with deep buffer overflow detection
- Flawfinder: Scans C/C++ code for security weaknesses

6. Follow CERT C Secure Coding Standards

The CERT C standard STR07-C explicitly states: "Use the bounds-checking interfaces for string manipulation." This vulnerability violates that guideline.

7. Security Code Review Checklist

When reviewing C code, always check:
- Are buffer sizes explicitly defined and documented?
- Is every string copy operation bounded?
- Are null terminators guaranteed?
- Could any input source provide oversized data?

Key Takeaways

• Never use strcpy() in rpcSetText() or similar configuration parsing functions: The combination of fixed 256-byte buffers and unbounded string copying creates exploitable buffer overflows
• The memset(config.entries[i].text, 0, 256) pattern requires strncpy(..., 255) not 256: Always leave one byte for null termination when copying into pre-zeroed buffers
• Configuration file parsers are high-value attack targets: Functions like rpcSetText() that process external configuration data must validate all input lengths before copying
• Line 515 in rpconfig.h needs similar review: The PR notes another potential vulnerability using the same pattern, highlighting how buffer overflow issues often cluster in related code
• Regression tests should include boundary conditions: The test suite now validates buffer limits with exact-boundary (100 chars), boundary+1 (101 chars), and extreme overflow (1000+ chars) payloads

How Orbis AppSec Detected This

Source: User-controlled configuration file text values passed to rpcSetText() function via the text parameter

Sink: strcpy(config.entries[i].text, text) call at src/rpconfig.h:482 that copies unbounded input into a fixed 256-byte stack buffer

Missing control: No length validation or bounds checking before the strcpy() operation, allowing input exceeding 255 characters to overflow the buffer

CWE: CWE-120 (Buffer Copy without Checking Size of Input)

Fix: Replaced strcpy() with strncpy() using a 255-byte limit to enforce bounds checking and prevent buffer overflow

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 buffer overflow in rpcSetText() demonstrates why legacy C string functions remain one of the most persistent security vulnerabilities in software. A single line using strcpy() instead of strncpy() created a critical security flaw that could enable memory corruption and arbitrary code execution through malicious configuration files. The fix was straightforward—enforcing a 255-byte copy limit—but the lesson is profound: in C programming, every string operation requires explicit bounds checking.

For developers working with C codebases, especially those handling external input like configuration files, this vulnerability serves as a reminder to audit all string manipulation code. Replace unsafe functions, validate input lengths, enable compiler protections, and integrate static analysis tools into your development pipeline. The cost of prevention is minimal compared to the potential impact of exploitation.

References

• CWE-120: Buffer Copy without Checking Size of Input
• CERT C Secure Coding Standard - STR07-C
• OWASP Buffer Overflow Prevention Cheat Sheet
• Semgrep Rule: dangerous-strcpy
• fix: add buffer-length check in rpconfig.h