Back to Blog
critical SEVERITY5 min read

How stack buffer overflow happens in C memcpy() with caller-controlled length and how to fix it

A critical stack buffer overflow vulnerability was discovered in GDI/Comdlg32.cpp where the `memcpy()` function used a caller-controlled `lStructSize` field without validation, allowing attackers to write beyond stack-allocated buffers. The fix applies a simple `min()` check across four affected dialog functions to ensure copy operations never exceed the destination buffer size.

O
By Orbis AppSec
•Published June 10, 2026•Reviewed June 10, 2026
#security#buffer-overflow#c-cpp#windows#cwe-120#memory-safety#code-review

Answer Summary

This is a stack buffer overflow vulnerability (CWE-120) in C/C++ Windows dialog code where `memcpy()` copies data using an attacker-controlled length field (`lStructSize`) without bounds checking. The fix wraps the copy length with `min(lpOpenFile->lStructSize, sizeof(OpenFile))` to ensure the copy never exceeds the stack-allocated destination buffer, preventing stack smashing attacks in the `GetOpenFileName` and `GetSaveFileName` wrapper functions.

Vulnerability at a Glance

cweCWE-120
fixClamp copy length to min(lStructSize, sizeof(destination))
riskRemote code execution via stack smashing
languageC/C++
root causeUsing caller-controlled lStructSize as memcpy length without validation
vulnerabilityStack Buffer Overflow

Introduction

In the GDI/Comdlg32.cpp file, which provides wrapper functions for Windows common dialog operations, a critical stack buffer overflow vulnerability lurked in four seemingly innocent memcpy() calls. The functions comdlg_GetOpenFileNameA, comdlg_GetOpenFileNameW, comdlg_GetSaveFileNameA, and comdlg_GetSaveFileNameW all shared the same dangerous pattern: they trusted a caller-supplied size field to determine how many bytes to copy onto the stack.

At lines 57, 80, 103, and 126, the code performed:

memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);

This single line is a textbook example of why you should never trust caller-controlled data for memory operations. The lStructSize field in the OPENFILENAME structure is meant to indicate the structure's size for versioning purposes, but nothing stops a malicious caller from setting it to an arbitrarily large value.

The Vulnerability Explained

What Made This Code Dangerous

The vulnerable pattern appeared in the comdlg_GetOpenFileNameA function at line 57:

BOOL WINAPI comdlg_GetOpenFileNameA(LPOPENFILENAMEA lpOpenFile)
{
    if (lpOpenFile && (lpOpenFile->Flags & OFN_ENABLEHOOK))
    {
        OPENFILENAMEA OpenFile = {};
        memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);  // VULNERABLE!
        UpdateOpenFileNameStruct(OpenFile);
        return GetOpenFileName(&OpenFile);
    }
    // ...
}

Here's the problem: OpenFile is a stack-allocated structure with a fixed size determined at compile time by sizeof(OPENFILENAMEA). However, the memcpy() call uses lpOpenFile->lStructSize as the copy length—a value completely controlled by the caller.

Attack Scenario

An attacker could craft a malicious OPENFILENAME structure like this:

// Attacker's code
std::vector<uint8_t> malicious_buffer(0x1000, 0x41);  // 4KB of 'A's
OPENFILENAMEA* crafted = reinterpret_cast<OPENFILENAMEA*>(malicious_buffer.data());
crafted->lStructSize = 0x1000;  // Claim the structure is 4KB
crafted->Flags = OFN_ENABLEHOOK;  // Trigger the vulnerable code path

// Embed shellcode or ROP gadgets at the right offset to overwrite return address
// ...

comdlg_GetOpenFileNameA(crafted);  // BOOM - stack smash

When memcpy() executes with a 4KB length but only a ~100-byte destination buffer, it writes far beyond OpenFile, overwriting:
- Other local variables
- Saved frame pointer
- Return address (critical for exploitation)
- Potentially reaching into adjacent stack frames

This is a classic stack smashing attack that can lead to arbitrary code execution.

Why This Pattern Repeated Four Times

The same vulnerable pattern existed in all four dialog wrapper functions:
- comdlg_GetOpenFileNameA (line 57) - ANSI version of file open
- comdlg_GetOpenFileNameW (line 80) - Unicode version of file open
- comdlg_GetSaveFileNameA (line 103) - ANSI version of file save
- comdlg_GetSaveFileNameW (line 126) - Unicode version of file save

Copy-paste programming propagated the vulnerability across all variants of the dialog functions.

The Fix

The fix is elegant in its simplicity: clamp the copy length to never exceed the destination buffer size.

Before (Vulnerable)

memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);

After (Fixed)

memcpy(&OpenFile, lpOpenFile, min(lpOpenFile->lStructSize, sizeof(OpenFile)));

The min() macro ensures that even if lStructSize is set to a billion bytes, the actual copy will never exceed sizeof(OpenFile)—the exact size of the stack-allocated destination buffer.

Applied Across All Four Locations

The fix was applied consistently to all vulnerable call sites:

Line 57 (GetOpenFileNameA):

-       memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);
+       memcpy(&OpenFile, lpOpenFile, min(lpOpenFile->lStructSize, sizeof(OpenFile)));

Line 80 (GetOpenFileNameW):

-       memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);
+       memcpy(&OpenFile, lpOpenFile, min(lpOpenFile->lStructSize, sizeof(OpenFile)));

Line 103 (GetSaveFileNameA):

-       memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);
+       memcpy(&OpenFile, lpOpenFile, min(lpOpenFile->lStructSize, sizeof(OpenFile)));

Line 126 (GetSaveFileNameW):

-       memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize);
+       memcpy(&OpenFile, lpOpenFile, min(lpOpenFile->lStructSize, sizeof(OpenFile)));

Why This Fix Works

The security invariant is now enforced: buffer writes never exceed the declared destination size. Even with a maliciously crafted lStructSize of 0xFFFFFFFF, the copy is bounded to exactly sizeof(OPENFILENAME) bytes—safe for the stack-allocated buffer.

Prevention & Best Practices

1. Never Trust Caller-Supplied Lengths

Any size, length, or count field that comes from external input (including structure fields) must be validated:

// Bad
memcpy(dest, src, untrusted_length);

// Good
memcpy(dest, src, min(untrusted_length, sizeof(dest)));

2. Use Safer Alternatives When Available

Modern C/C++ provides safer alternatives:
- memcpy_s() (C11 Annex K) - requires explicit destination size
- std::copy_n() with bounds checking in C++
- Platform-specific safe functions like StringCchCopy() on Windows

3. Enable Compiler Protections

While not a substitute for secure code, enable all available mitigations:
- Stack canaries (/GS on MSVC, -fstack-protector on GCC/Clang)
- ASLR and DEP
- Control Flow Guard (CFG) on Windows

4. Static Analysis Integration

Tools like Semgrep, Coverity, and PVS-Studio can detect this pattern. A simple Semgrep rule might look for memcpy calls where the length argument is a structure field access.

Key Takeaways

  • Never use lStructSize or similar caller-controlled fields directly as memcpy() lengths — always clamp to sizeof(destination)
  • The min() pattern is your friend — min(user_length, sizeof(buffer)) is a defensive programming essential
  • Copy-paste vulnerabilities multiply — when fixing one instance, search for the same pattern throughout the codebase (this fix addressed 4 locations)
  • Stack-allocated buffers are especially dangerous — overflow can directly corrupt return addresses, enabling RCE
  • Regression tests should encode security invariants — the added test suite verifies the fix with adversarial inputs including sizeof(OPENFILENAME) * 10 and 0xFFFF

How Orbis AppSec Detected This

  • Source: The lpOpenFile->lStructSize field in caller-provided OPENFILENAME structures
  • Sink: memcpy(&OpenFile, lpOpenFile, lpOpenFile->lStructSize) at lines 57, 80, 103, and 126 in GDI/Comdlg32.cpp
  • Missing control: No validation that lStructSize does not exceed sizeof(OpenFile) before the copy operation
  • CWE: CWE-120 (Buffer Copy without Checking Size of Input)
  • Fix: Added min(lpOpenFile->lStructSize, sizeof(OpenFile)) to clamp the copy length to the destination buffer size

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 fundamental truth in systems programming: never trust external data for memory operations. The lStructSize field, while seemingly innocuous as a versioning mechanism, became an attack vector when used directly as a memcpy() length.

The fix—a simple min() call—is both minimal and complete. It preserves functionality for legitimate callers while neutralizing malicious inputs. More importantly, the accompanying regression test suite ensures this security property is maintained as the code evolves.

When working with C/C++ code that handles external structures, always ask: "What happens if this size field is larger than I expect?" If the answer involves writing past buffer boundaries, you've found a vulnerability waiting to happen.

References

Frequently Asked Questions

What is a stack buffer overflow?

A stack buffer overflow occurs when a program writes more data to a stack-allocated buffer than it can hold, potentially overwriting return addresses and enabling arbitrary code execution.

How do you prevent stack buffer overflow in C?

Always validate buffer lengths before copy operations, use bounded functions like strncpy() or memcpy_s(), and never trust caller-supplied size values without clamping them to destination buffer sizes.

What CWE is stack buffer overflow?

CWE-120 (Buffer Copy without Checking Size of Input) covers this class of vulnerability where copy operations use unchecked input lengths.

Is using sizeof() enough to prevent buffer overflow?

No, sizeof() alone doesn't prevent overflow—you must use it to bound the actual copy length, typically with min(user_length, sizeof(buffer)) or equivalent validation.

Can static analysis detect stack buffer overflow?

Yes, static analyzers can detect patterns where memcpy/strcpy use untrusted length values, though complex data flows may require taint tracking to identify all vulnerable paths.

View the Security Fix

Check out the pull request that fixed this vulnerability

View PR #564
View Repository →

Related Articles

medium

How insecure update manifest parsing happens in C++ UpdateHelper.cpp and how to fix it

TrafficMonitor's software update mechanism in `UpdateHelper.cpp` fetched and parsed update manifests from remote servers without validating the version string or enforcing trusted download URLs, leaving users exposed to man-in-the-middle (MITM) attacks. An attacker on the same network could intercept the update channel and inject a malicious binary under a crafted version string or an HTTP download link pointing to attacker-controlled infrastructure. The fix adds strict version-string sanitizati

high

How integer overflow in malloc happens in C bipartite matching and how to fix it

A high-severity integer overflow vulnerability was discovered in the bipartite matching algorithm implementation where unchecked multiplication operations for memory allocation could wrap around, causing undersized buffer allocations and subsequent heap overflow. The fix replaces vulnerable `malloc(sizeof(int) * V)` patterns with safe `calloc(V, sizeof(int))` calls and adds proper bounds validation to prevent exploitation.

high

How integer truncation heap overflow happens in C++ UEFI ACPI parsing and how to fix it

A high-severity integer truncation vulnerability was discovered in `Mobility.Uefi.Acpi.cpp` where heap allocation sizes were stored in a 16-bit integer (`MO_UINT16`), causing silent truncation when the computed size exceeded 65535 bytes. This led to undersized heap allocations followed by out-of-bounds writes, exploitable by an attacker who can influence ACPI SRAT table contents in virtualized environments. The fix promotes the size variable to `MO_UINTN` (platform-native width) to prevent trunc

critical

How API key exposure in configuration files happens in TOML config and how to fix it

A critical security vulnerability in `commands/webperf.toml` allowed API keys to be hardcoded directly in configuration files, creating a credential exposure risk. The documentation on line 11 suggested developers could provide `CRUX_API_KEY` or `GOOGLE_API_KEY` directly in the config, which could lead to these sensitive credentials being committed to version control or exposed in logs. The fix updated the documentation to explicitly require environment variables and warn against hardcoding cred

high

How path traversal happens in Ruby YARD server and how to fix it

A high-severity path traversal vulnerability (CVE-2026-41493) in YARD versions prior to 0.9.42 allowed attackers to read arbitrary files from servers running `yard server`. This fix upgrades the yard gem from 0.9.26 to 0.9.42 in the Gemfile and Gemfile.lock, closing a dangerous information disclosure vector that could expose configuration files, credentials, and source code.

high

How buffer overflow via sprintf() happens in C networking code and how to fix it

A high-severity buffer overflow vulnerability was discovered in `profile.c` where `sprintf()` was used to format server addresses without any bounds checking. An attacker who could influence the `SERVER_BASE_PORT` value or trigger integer overflow in the port calculation could write beyond the `server_address` buffer. The fix replaces `sprintf()` with `snprintf()` using explicit buffer size limits at both call sites (lines 99 and 220).