Skip to content

Fix bugs: memory leak, missing relocations, dead code, wrong section address#26

Closed
tishion wants to merge 2 commits into
masterfrom
fix-bugs-memory-leak-and-relocation
Closed

Fix bugs: memory leak, missing relocations, dead code, wrong section address#26
tishion wants to merge 2 commits into
masterfrom
fix-bugs-memory-leak-and-relocation

Conversation

@tishion
Copy link
Copy Markdown
Owner

@tishion tishion commented Apr 15, 2026

Summary

Fixed 4 bugs in the mmLoader codebase:

Bug 1: Memory Leak in InitApiTable()

  • Location: src/mmLoader/mmLoader.c, line ~491
  • Issue: When InitApiTable() fails inside the do-while loop, it returns NULL but never frees the allocated pApis table
  • Fix: Added pfnGlobalFree(pApis) before returning NULL on failure path

Bug 2: Missing Relocation Types in RelocateModuleBase()

  • Location: src/mmLoader/mmLoader.c, lines ~710-735
  • Issue: Only handles IMAGE_REL_BASED_HIGHLOW (32-bit) and IMAGE_REL_BASED_DIR64 (64-bit), missing IMAGE_REL_BASED_HIGH and IMAGE_REL_BASED_LOW which can cause incorrect relocations
  • Fix: Added handling for HIGH and LOW relocation types in 32-bit builds

Bug 3: Dead Code in IsValidPEFormat()

  • Location: src/mmLoader/mmLoader.c, lines ~529-530
  • Issue: The else br = FALSE; is outside both #ifdef branches - unreachable dead code that doesn't properly handle non-standard machine types
  • Fix: Removed the unreachable else branch

Bug 4: Wrong Section Base Address in SetMemProtectStatus()

  • Location: src/mmLoader/mmLoader.c, line ~864 (previously ~851)
  • Issue: Uses pImageSectionHeader[idxSection].Misc.PhysicalAddress which was overwritten at line ~661 with the destination address, not the original file offset. This caused incorrect memory protection settings.
  • Fix: Changed to compute section base directly using pMemModule->lpBase + pImageSectionHeader[idxSection].VirtualAddress
  • Bonus: Removed now-unused ulBaseHigh variable

@tishion can click here to continue refining the PR

…address

- Fix memory leak in InitApiTable() - free pApis on failure
- Add IMAGE_REL_BASED_HIGH and IMAGE_REL_BASED_LOW handling in RelocateModuleBase()
- Remove dead code (unreachable else branch) in IsValidPEFormat()
- Fix SetMemProtectStatus to use lpBase + VirtualAddress instead of overwritten PhysicalAddress
- Remove unused ulBaseHigh variable
@tishion tishion closed this Apr 18, 2026
@tishion tishion deleted the fix-bugs-memory-leak-and-relocation branch April 18, 2026 07:21
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