Heap Corruption via Unchecked memcpy: How Integer Overflow Bugs Corrupt Memory in Windows File Operations

Introduction

Buffer overflows are among the oldest and most dangerous vulnerability classes in systems programming — and they keep showing up in production code. This post breaks down a critical-severity heap buffer overflow discovered in phlib/nativefile.c, a native file-operation helper used in a Windows application built on the Process Hacker / SystemInformer codebase.

The root cause is deceptively simple: arithmetic used to calculate allocation sizes was not protected against integer overflow, and the lengths of caller-supplied strings were never validated before being copied into those allocations. The result? A carefully crafted filename or extended-attribute (EA) name could silently corrupt heap memory adjacent to the allocated buffer.

If you write C or C++ code that touches the Windows Native API — or any low-level file I/O — this vulnerability pattern is one you need to recognize on sight.

The Vulnerability Explained

What Is a Heap Buffer Overflow?

A heap buffer overflow occurs when code writes data beyond the end of a heap-allocated buffer. Unlike stack overflows (which famously overwrite return addresses), heap overflows corrupt adjacent heap metadata or other live allocations. This can lead to:

• Arbitrary code execution if an attacker can control what gets written and where
• Privilege escalation in kernel-adjacent or privileged processes
• Denial of service through process crashes or corrupted state
• Information disclosure if heap layout leaks sensitive data from adjacent allocations

CWE Reference: This vulnerability maps to CWE-122: Heap-based Buffer Overflow and CWE-190: Integer Overflow or Wraparound.

The Vulnerable Code: Two Separate Sites

The vulnerability manifested in two places inside phlib/nativefile.c.

Site 1 — PhMoveFile: Rename Information Buffer

// VULNERABLE — before the fix
renameInfoLength = sizeof(FILE_RENAME_INFORMATION) + fileNameLength + sizeof(UNICODE_NULL);
renameInfo = PhAllocateStack(renameInfoLength);

FILE_RENAME_INFORMATION is a Windows Native API structure used when renaming files via NtSetInformationFile. It has a flexible trailing array for the new filename. The code calculates the required buffer size by adding three values together using plain C arithmetic:

• sizeof(FILE_RENAME_INFORMATION) — fixed structure size
• fileNameLength — caller-supplied, derived from a UNICODE_STRING
• sizeof(UNICODE_NULL) — a two-byte null terminator

The problem: If fileNameLength is close to ULONG_MAX (0xFFFFFFFF), the addition wraps around to a small number. The allocation succeeds — allocating a tiny buffer — but the subsequent memcpy writes the full, large filename into it, blowing past the buffer boundary and corrupting the heap.

Site 2 — PhSetFileExtendedAttributes: EA Information Buffer

// VULNERABLE — before the fix
infoLength = sizeof(FILE_FULL_EA_INFORMATION) + (ULONG)Name->Length + sizeof(ANSI_NULL);
if (Value) infoLength += (ULONG)Value->Length + sizeof(ANSI_NULL);

FILE_FULL_EA_INFORMATION holds extended attributes for NTFS files. The same pattern repeats: unchecked addition of caller-supplied Name->Length and Value->Length values. A SIZE_T-to-ULONG cast ((ULONG)Name->Length) silently truncates on 64-bit systems if Name->Length exceeds 32 bits, and the additions that follow can still overflow.

There's a secondary issue here too: the EA name length is stored in a UCHAR field (EaNameLength, max 255) and the value length in a USHORT field (EaValueLength, max 65535). If the caller provides lengths that don't fit in those narrower types, the truncated values written into the structure will be inconsistent with the actual data copied — a recipe for downstream memory corruption.

Attack Scenario

Consider a privileged file manager or security tool that calls PhMoveFile with a destination path derived from user input (a rename dialog, a command-line argument, or a network-sourced path). An attacker who can influence the destination filename can supply a string whose byte length, when added to sizeof(FILE_RENAME_INFORMATION), wraps around to a small value. The allocator hands back a tiny buffer; the memcpy copies gigabytes (or just a few extra bytes) past it.

In practice, exploitability depends on:
- Whether the attacker controls input to these functions
- The heap layout at the time of the overflow
- Whether the process runs with elevated privileges (common for file utilities)

Even without a full exploit chain, a reliable crash (denial of service) is trivially achievable once this primitive is in hand.

The Fix

The fix replaces every unchecked arithmetic expression with Windows safe integer helper functions that return an error status on overflow, and adds narrowing conversion checks before assigning lengths to smaller-typed struct fields.

Fix 1 — PhMoveFile: Safe Size Calculation

// FIXED — after the patch
status = RtlULongAdd(sizeof(FILE_RENAME_INFORMATION), fileNameLength, &renameInfoLength);
if (!NT_SUCCESS(status))
    goto CleanupExit;

status = RtlULongAdd(renameInfoLength, sizeof(UNICODE_NULL), &renameInfoLength);
if (!NT_SUCCESS(status))
    goto CleanupExit;

renameInfo = PhAllocateStack(renameInfoLength);
if (!renameInfo) return STATUS_NO_MEMORY;

RtlULongAdd (from <intsafe.h>) performs the addition and returns STATUS_INTEGER_OVERFLOW if the result would exceed ULONG_MAX. The code now bails out cleanly before any allocation occurs, rather than allocating a dangerously undersized buffer.

Fix 2 — PhSetFileExtendedAttributes: Safe Size Calculation + Narrowing Checks

// FIXED — after the patch

// Step 1: Validate Name->Length fits in ULONG, then in UCHAR
status = RtlSIZETToULong(Name->Length, &nameLength);
if (!NT_SUCCESS(status))
    return status;
status = RtlULongToUChar(nameLength, &eaNameLength);  // must fit in UCHAR (max 255)
if (!NT_SUCCESS(status))
    return status;

// Step 2: Build infoLength safely
status = RtlULongAdd(sizeof(FILE_FULL_EA_INFORMATION), nameLength, &infoLength);
if (!NT_SUCCESS(status))
    return status;
status = RtlULongAdd(infoLength, sizeof(ANSI_NULL), &infoLength);
if (!NT_SUCCESS(status))
    return status;

// Step 3: Validate Value->Length fits in ULONG, then in USHORT
if (Value)
{
    status = RtlSIZETToULong(Value->Length, &valueLength);
    if (!NT_SUCCESS(status))
        return status;
    status = RtlULongToUShort(valueLength, &eaValueLength);  // must fit in USHORT (max 65535)
    if (!NT_SUCCESS(status))
        return status;

    status = RtlULongAdd(infoLength, valueLength, &infoLength);
    // ... continues
}

This fix addresses three distinct hazards in one location:
1. Integer overflow in the size arithmetic (via RtlULongAdd)
2. Truncating cast from SIZE_T to ULONG (via RtlSIZETToULong)
3. Narrowing mismatch between logical length and struct field width (via RtlULongToUChar / RtlULongToUShort)

Why RtlULongAdd and Friends?

Microsoft ships a comprehensive safe-integer library in <intsafe.h> specifically to address this class of bug. Each function:
- Performs the operation
- Checks for overflow/underflow
- Returns S_OK / STATUS_INTEGER_OVERFLOW accordingly
- Writes the result only if safe

Using these functions is the Windows-idiomatic equivalent of Rust's checked_add() or C++'s std::numeric_limits checks.

Conclusion

This vulnerability is a textbook example of why integer arithmetic in allocation size calculations is a security-critical operation, not a mundane implementation detail. The original code looked completely reasonable at a glance — three values added together to size a buffer — but the absence of overflow checks turned it into a heap corruption primitive.

The fix is equally instructive: it doesn't require a dramatic architectural change. Replacing three lines of arithmetic with a handful of RtlULongAdd calls and narrowing-conversion checks is all it takes to close the vulnerability. The Windows safe-integer API exists precisely for this purpose, and using it consistently is a low-friction habit that pays significant security dividends.

Key takeaways for developers:

1. Every addition or multiplication that feeds into an allocation size is a potential integer overflow — treat it that way.
2. Every narrowing cast (wider int → narrower int) is a potential truncation bug — validate before casting, don't cast and hope.
3. Use platform-provided safe integer libraries (<intsafe.h> on Windows, __builtin_add_overflow on GCC/Clang, checked_add in Rust) rather than rolling your own checks.
4. Run AddressSanitizer and UBSan in your CI pipeline — they catch these bugs cheaply before they reach production.
5. Automated security scanning (as used to detect this issue) is a valuable layer of defense, but it works best when paired with developer education about why these patterns are dangerous.

Secure coding is a discipline, not a checklist. Understanding the mechanism of vulnerabilities like this one is what allows you to spot novel instances of the same pattern in code that no scanner has seen before.

Fixed by the OrbisAI Security automated remediation pipeline. Learn more at orbisappsec.com.