How buffer overflow happens in C++ and how to fix it

Introduction

The hmlangw.cpp file implements the command handling logic for the HM-LGW gateway device firmware, including a helper function called create_hex_string() that formats raw binary data into a human-readable hex-dump string — the kind of debug output you'd see on a serial console or in a log line like 01 a2 ff 3c. This function is called throughout the codebase (the PR notes at least 17 similar call sites at lines 83, 84, 89, 143, 167, 226, and more) whenever incoming or outgoing byte buffers need to be rendered as hex for logging or protocol framing.

The problem: create_hex_string() hardcoded its loop to run exactly 16 times, writing 3 bytes per iteration into a caller-supplied target buffer, using sprintf() — a function with zero awareness of how big that buffer actually is. If the destination buffer passed in was smaller than 48 bytes (16 × 3), the loop would happily walk right past the end of it, corrupting adjacent stack or heap memory. Given that this function is invoked to format data arriving over a serial interface or network socket, an attacker who can influence the size or content of that input has a real path to memory corruption.

The Vulnerability Explained

Here's the vulnerable code before the fix:

static char* create_hex_string( const char* source, int length, char* target, int size )
{
    ...
    if ( source )
    {
          current = target;
        int i = 16;
          while ( i-- )
        {
            length--;

            if(length >= 0)
            {
                    value = *source++;
                    sprintf( current, " %02x", value );
                    current += 3;
            }
            else
            {
                sprintf( current, "   " );
                current += 3;
            }
          }

Notice what's happening: size is passed into the function specifically so it can determine how much room is actually available in target. But the loop counter i is set to a fixed 16, completely ignoring size. Each iteration:

1. Calls sprintf(current, " %02x", value) or sprintf(current, " ") — neither call knows the remaining capacity of current.
2. Advances the pointer with current += 3, assuming each write was exactly 3 bytes (true for well-formed hex, but sprintf provides no guarantee or check).

If any caller invoked create_hex_string() with a target buffer smaller than 48 bytes plus the null terminator — which is entirely plausible given the function is called from at least 17 different sites with varying buffer sizes like hex[100] truncated or reused for smaller framing — the function would write past the buffer boundary every time, regardless of the size argument's actual value.

Attack scenario: Consider a caller that passes a target buffer sized for a shorter protocol field (say, an 8-byte hex field) but the same create_hex_string() function still executes all 16 iterations because the loop bound was never tied to size. An attacker who controls the length or content of the source data streaming in over the serial line or TCP socket could force this mismatch, causing a stack or heap buffer overflow. Depending on stack layout and adjacent variables, this could corrupt return addresses, function pointers, or other security-critical state — turning a logging helper into a potential remote code execution primitive on embedded gateway hardware.

The Fix

The PR makes two coordinated changes to create_hex_string() at the lines flagged in the vulnerability report (line 83 and its neighbors):

1. Bound the loop count to the actual buffer size, instead of blindly assuming 16 iterations fit:

int maxIterations = ( size - 1 ) / 3;
int i = maxIterations < 16 ? maxIterations : 16;

This computes how many 3-byte hex chunks can actually fit in size bytes (reserving 1 byte for the null terminator), and takes the smaller of that value and the original design maximum of 16. Now the loop can never write more than the buffer can hold, no matter what size the caller passes in.

2. Replace unbounded sprintf with bounds-checked snprintf:

// Before
sprintf( current, " %02x", value );
current += 3;
...
sprintf( current, "   " );
current += 3;

// After
snprintf( current, 4, " %02x", value );
current += 3;
...
snprintf( current, 4, "   " );
current += 3;

The 4 passed to snprintf accounts for the 3 visible characters (" xx" or " ") plus the null terminator, matching the current += 3 pointer arithmetic exactly. Even if something unexpected happened upstream, snprintf guarantees it will never write beyond those 4 bytes at current, eliminating the possibility of the write itself overflowing — while the loop-bound fix eliminates the possibility of current walking past target in the first place.

Together, these two changes address the vulnerability defense-in-depth style: the loop bound stops the pointer from ever exceeding the buffer, and snprintf stops any single write from exceeding its slot even under unexpected conditions. The PR also bumped VERSION from 1.1.0 to 1.1.1 and updated HMLANGW_VERSION in the buildroot .mk file accordingly, since this is a behavior-affecting patch to shipped firmware.

Prevention & Best Practices

• Never hardcode loop bounds independent of buffer size. Any function that accepts both a destination buffer and its size must derive all internal iteration counts from that size, not from a separate assumed constant like 16.
• Replace sprintf with snprintf everywhere, especially in C/C++ code that formats attacker-influenced data. snprintf's explicit length argument is cheap insurance against exactly this class of bug.
• Audit repeated patterns across a codebase. The PR description flags 17+ additional call sites using the same "fixed iteration + sprintf + pointer bump" pattern — this suggests it's worth grepping for sprintf( and unconditional loop counters across the whole hmlangw.cpp file and similar embedded C++ modules.
• Use static analysis tools (e.g., Cppcheck, Clang Static Analyzer, Semgrep) configured to flag sprintf usage and to warn when a size parameter is accepted but not used consistently throughout a function.
• Reference CWE-120 (Buffer Copy without Checking Size of Input) and CWE-787 (Out-of-bounds Write) when triaging findings like this — both map directly to the root cause here.

Key Takeaways

• create_hex_string() accepted a size parameter but ignored it when setting the loop bound — always wire size parameters into every bound-related decision inside a function, not just some of them.
• The fix's maxIterations = (size - 1) / 3 formula is a direct, buffer-size-aware replacement for the previous hardcoded int i = 16.
• sprintf( current, " %02x", value ) and sprintf( current, " " ) were both replaced with snprintf(current, 4, ...), matching the current += 3 pointer stride exactly.
• This function is called from 17+ locations in hmlangw.cpp (lines 84, 89, 143, 167, 226, and more) — a single vulnerable helper had a wide blast radius across the file.
• Since source data ultimately originates from a serial interface or network socket, this bug was reachable by any attacker who can influence traffic to the HM-LGW gateway.

How Orbis AppSec Detected This

• Source: Data read from the serial interface or network socket and passed into create_hex_string() as the source/length parameters.
• Sink: The unbounded sprintf( current, " %02x", value ) and sprintf( current, " " ) calls at hmlangw.cpp:83 writing into the fixed-size hex[100] buffer via the current pointer.
• Missing control: No bounds check tying the loop's iteration count or each sprintf write to the size parameter of the destination buffer.
• CWE: CWE-120 (Buffer Copy without Checking Size of Input) / CWE-787 (Out-of-bounds Write).
• Fix: Cap loop iterations with maxIterations = (size - 1) / 3 and replace all sprintf calls with snprintf(current, 4, ...) to enforce a hard 4-byte write limit per iteration.

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 is a textbook reminder that accepting a size parameter isn't the same as using it. create_hex_string() in hmlangw.cpp looked safe at a glance — it even tracked size — but the actual loop bound and every sprintf call inside it ignored that value entirely, leaving a 48-byte write against buffers that could be smaller. The fix is small but precise: derive the loop count from the real buffer capacity and let snprintf enforce hard limits on every individual write. For any embedded or network-facing C++ code handling attacker-influenced input, treating buffer size as a first-class constraint — not just a variable you happen to have in scope — is the difference between a clean hex dump and a critical memory corruption bug.

References

• CWE-120: Buffer Copy without Checking Size of Input — https://cwe.mitre.org/data/definitions/120.html
• CWE-787: Out-of-bounds Write — https://cwe.mitre.org/data/definitions/787.html
• OWASP C-Based Toolchain Hardening Cheat Sheet — https://cheatsheetseries.owasp.org/cheatsheets/C-Based_Toolchain_Hardening_Cheat_Sheet.html
• cppreference: std::snprintf — https://en.cppreference.com/w/cpp/io/c/fprintf
• Semgrep rule search for unsafe sprintf usage — https://semgrep.dev/r?q=sprintf
• fix: use snprintf in hmlangw.cpp