Critical Stack Buffer Overflow Fixed in sgl_log.c: What You Need to Know

Severity: 🔴 Critical | CWE: CWE-120 | File: source/core/sgl_log.c:48

Introduction

Buffer overflows are one of the oldest vulnerabilities in software security — and one of the most dangerous. Despite decades of awareness, tooling improvements, and compiler warnings, they continue to appear in production codebases, particularly in C and C++ code that prioritizes performance or was written before modern safety practices became standard.

This post covers a critical stack buffer overflow that was recently discovered and patched in source/core/sgl_log.c. The vulnerability stemmed from an unsafe call to strcpy() followed by an unvalidated memcpy() into a fixed-size stack buffer — a textbook example of CWE-120 (Buffer Copy Without Checking Size of Input).

If you write C code, maintain legacy systems, or are simply curious about how memory corruption vulnerabilities work, this post is for you.

The Vulnerability Explained

What Is a Stack Buffer Overflow?

When a program declares a local variable inside a function, that variable lives on the stack — a region of memory that also stores the function's return address (where execution should resume after the function completes) and other critical metadata.

A stack buffer overflow occurs when a program writes more data into a stack-allocated buffer than it was designed to hold. The excess data spills over into adjacent memory regions, potentially overwriting the return address or other sensitive values.

What Was Wrong in sgl_log.c?

The vulnerable code existed in the logging module at line 48. Here's a simplified representation of the problematic pattern:

// ⚠️ VULNERABLE CODE (simplified for illustration)
void sgl_log(const char *level, const char *message) {
    char buffer[64];  // Fixed-size stack buffer

    // Line 48: No bounds check — copies level into fixed buffer
    strcpy(buffer, level);

    // Line 56: Copies message at an offset WITHOUT validating remaining capacity
    int tail = strlen(level);
    memcpy(buffer + tail, message, strlen(message));

    // ... rest of logging logic
}

Let's break down exactly why this is dangerous:

1. strcpy(buffer, level) at line 48: The strcpy function copies bytes from level into buffer until it hits a null terminator (\0). It performs zero bounds checking. If level is longer than 63 characters (leaving room for the null terminator in a 64-byte buffer), it will overflow the buffer immediately.

2. memcpy(buffer + tail, ...) at line 56: Even if level fits, the subsequent memcpy copies message into the remaining space in the buffer — but without calculating how much space actually remains. If strlen(level) + strlen(message) > 64, the write goes out of bounds.

3. The combined effect: An attacker (or even just unexpected input) providing a sufficiently long level or message string causes writes beyond the buffer boundary, corrupting the stack.

How Could It Be Exploited?

In a classic stack smashing attack, an adversary crafts input that:

1. Fills the buffer with arbitrary data
2. Overwrites the return address with an address of their choosing
3. Redirects execution to attacker-controlled code (shellcode, a ROP chain, or an existing function)

Here's a conceptual attack scenario:

Stack layout (before overflow):
┌─────────────────────┐  ← High addresses
│   Return Address    │  ← Where execution goes after sgl_log() returns
│   Saved Registers   │
│   buffer[64]        │  ← Our 64-byte buffer starts here
└─────────────────────┘  ← Low addresses

Stack layout (after overflow with 128-byte input):
┌─────────────────────┐
│  0x41414141 (AAAA)  │  ← Return address OVERWRITTEN by attacker input
│  AAAAAAAAAAAAAAAA   │  ← Saved registers corrupted
│  AAAAAAAAAAAAAAAA   │  ← Buffer filled with attacker data
└─────────────────────┘

When the function returns, instead of jumping back to the legitimate caller, the CPU jumps to wherever the attacker specified — potentially executing malicious code.

Real-World Impact

The consequences of a successfully exploited stack buffer overflow can include:

• Arbitrary code execution — the attacker runs any code they want with the privileges of the affected process
• Privilege escalation — if the logging module runs with elevated permissions, an attacker could gain root or SYSTEM access
• Denial of Service — even without full exploitation, corrupted stack data causes crashes and service outages
• Data exfiltration — attackers can pivot from code execution to accessing sensitive data

This is why the vulnerability is rated Critical.

The Fix

What Changed

The fix replaces the unsafe strcpy/memcpy pattern with bounds-aware alternatives and proper length validation. Here's the corrected approach:

// ✅ FIXED CODE (simplified for illustration)
void sgl_log(const char *level, const char *message) {
    char buffer[64];

    // Use strncpy with explicit size limit
    strncpy(buffer, level, sizeof(buffer) - 1);
    buffer[sizeof(buffer) - 1] = '\0';  // Ensure null termination

    // Calculate remaining capacity BEFORE copying
    size_t level_len = strlen(buffer);
    size_t remaining = sizeof(buffer) - level_len - 1;

    if (remaining > 0 && message != NULL) {
        strncat(buffer, message, remaining);
    }

    // ... rest of logging logic
}

Alternatively, using snprintf — the gold standard for safe formatted string construction in C:

// ✅ EVEN BETTER: Use snprintf for safe, bounded formatting
void sgl_log(const char *level, const char *message) {
    char buffer[64];

    // snprintf ALWAYS respects the buffer size limit
    int written = snprintf(buffer, sizeof(buffer), "%s%s", level, message);

    if (written < 0) {
        // Handle encoding error
        return;
    }
    if ((size_t)written >= sizeof(buffer)) {
        // Output was truncated — log a warning or handle appropriately
    }

    // ... rest of logging logic
}

Why This Fix Works

The key improvements are:
1. Explicit size limits prevent writes beyond the buffer boundary
2. Remaining capacity calculation ensures the second write knows how much space is available
3. Null termination is guaranteed, preventing read overflows downstream

Conclusion

The stack buffer overflow in sgl_log.c is a stark reminder that even "boring" utility code like logging modules can harbor critical vulnerabilities. A single unsafe strcpy call — a function that has been known to be dangerous for over 40 years — was enough to introduce a potentially exploitable memory corruption bug.

Key Takeaways

✅ Never use unbounded string functions (strcpy, strcat, sprintf, gets) in C code
✅ Always validate input length before copying into fixed-size buffers
✅ Calculate remaining capacity before every write into a shared buffer
✅ Use snprintf as your default for safe string construction
✅ Enable compiler hardening flags as a safety net
✅ Integrate static analysis into your CI/CD pipeline — don't rely on manual review alone
✅ Consider memory-safe languages for new projects where C isn't strictly required

Buffer overflows are preventable. With the right tools, habits, and code review practices, your codebase doesn't have to be the next cautionary tale.

This vulnerability was identified and fixed as part of an automated security scanning process. Security fixes like this one are most effective when combined with developer education — understanding why a vulnerability exists is the best way to prevent the next one.

Have questions about secure C programming or memory safety? Drop them in the comments below.

References
- CWE-120: Buffer Copy Without Checking Size of Input
- SEI CERT C Coding Standard - String Rules
- OWASP Buffer Overflow Attack
- Clang AddressSanitizer Documentation