Skip to content

Fix memory corruption vulnerabilities in ntoskrnl from unsafe string operations#4

Draft
Copilot wants to merge 5 commits into
masterfrom
copilot/fix-memory-corruption-issues
Draft

Fix memory corruption vulnerabilities in ntoskrnl from unsafe string operations#4
Copilot wants to merge 5 commits into
masterfrom
copilot/fix-memory-corruption-issues

Conversation

Copy link
Copy Markdown

Copilot AI commented May 29, 2026

Bootloader-controlled data (device names, CPU identifiers) written to fixed-size buffers with sprintf/strcpy, and debugger infrastructure with unbounded vsprintf, create multiple buffer overflow and wild-write vulnerabilities. This PR systematically eliminates these patterns.

Changes

Unsafe string function replacement

  • 16× sprintf → snprintf with overflow checking
  • 5× strcpy → strncpy with explicit null-termination
  • vsprintf → vsnprintf in early kernel initialization paths

Example: arcname.c boot device initialization

// Before
CHAR Buffer[128];
sprintf(Buffer, "\\ArcName\\%s", LoaderBlock->ArcBootDeviceName);

// After
CHAR Buffer[128];
if (snprintf(Buffer, sizeof(Buffer), "\\ArcName\\%s", LoaderBlock->ArcBootDeviceName) >= (int)sizeof(Buffer)) {
    DPRINT1("ArcBootDeviceName path too long, truncated\n");
}

Return value validation for _vsnprintf

  • Fixed KdbpPrint casting -1 (truncation indicator) to ULONG, causing wild writes at Buffer[0xFFFFFFFF]
  • Fixed KdpDprintf casting -1 to USHORT (65535), causing OOB reads from 512-byte buffer
  • All _vsnprintf calls now properly check for negative returns and cap Length appropriately

Example: kdb_cli.c critical fix

// Before - wild write on truncation
Ret = _vsnprintf(Buffer, sizeof(Buffer) - 1, Format, ap);
Length = Ret;  // -1 cast to ULONG = 0xFFFFFFFF
Buffer[Length] = '\0';  // writes far outside buffer

// After
Ret = _vsnprintf(Buffer, sizeof(Buffer) - 1, Format, ap);
if (Ret < 0) Length = sizeof(Buffer) - 1;
else Length = (ULONG)Ret;

Buffer size validation with assertions

  • MmBuildMdlFromPages: validates MDL allocation includes sufficient PFN array space
  • IoBuildPartialMdl: validates target MDL can hold computed page count
  • Catches caller allocation errors at runtime rather than corrupting adjacent memory

Race condition mitigation

  • Increased debug function static buffers from 11 bytes (tight-fit for ULONG) to 16 bytes
  • Replaced sprintf with snprintf to prevent concurrent writes corrupting shared state

Files Modified

  • ntoskrnl/kdbg/kdb_cli.c (CRITICAL: wild write fix)
  • ntoskrnl/ke/arm/kiinit.c (HIGH: unbounded vsprintf)
  • ntoskrnl/kdbg/i386/i386-dis.c (HIGH: accumulating strcpy in disassembler)
  • ntoskrnl/io/iomgr/arcname.c, ex/init.c, io/iomgr/iomgr.c, fstub/fstubex.c (HIGH: 16 sprintf→snprintf)
  • ntoskrnl/kd64/kdprint.c, fstub/disksup.c, ps/psmgr.c, config/i386/cmhardwr.c (MEDIUM-LOW: safe string ops)
  • ntoskrnl/mm/pagefile.c, io/iomgr/iomdl.c, ps/query.c (validation/race condition fixes)

Copilot AI changed the title Fix memory corruption vulnerabilities in ntoskrnl Fix memory corruption vulnerabilities in ntoskrnl from unsafe string operations May 29, 2026
Copilot AI requested a review from tkreuzer May 29, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants