Bound .rsrc resource-tree parsing against crafted input

[christophwille]

ReadWin32Resources walks the PE resource directory tree with raw native pointer arithmetic over attacker-controlled offsets, counts and sizes, with no bounds checks, no recursion-depth limit and no cycle detection. The root section pointer comes from GetSectionData, whose length is read and then discarded, so every dereference is unbounded. Because "Save as project" reads these resources unconditionally, a crafted assembly can turn merely opening it into an uncatchable process kill or an out-of-bounds native read.

Three concrete vectors: a subdirectory entry pointing back at itself (or a long acyclic chain) recurses until the stack overflows; an inflated NumberOf*Entries walks the entry loop off the section end; and a data entry whose Size is up to 4 GB makes Buffer.MemoryCopy read far past the section base, faulting on an unmapped page or copying adjacent process memory into the byte[] that is then written to app.ico / app.manifest on disk. None of this is containable from the caller, since a StackOverflowException cannot be caught and there is no corrupted-state-exception handling in the codebase. It is the same class of bug as the single-file bundle signature fix in a154a7b.

The fix carries the section length (already returned by GetSectionData) alongside the root pointer and bounds-checks every offset, entry count, name-string length and data Size against it; it caps recursion depth and tracks visited directory offsets to break cycles. A hostile or truncated file now yields a bounded, partial tree instead of a crash, while well-formed resources parse exactly as before — the existing project-decompiler tests that read real icons and manifests continue to pass. As part of this, the parser no longer depends on the whole PEReader: it takes a small delegate that resolves a data RVA to a bounded pointer, which is also the seam the new tests exercise.

New tests build crafted .rsrc byte buffers over a pinned region and drive that seam directly: a self-referential subdirectory, a chain deeper than the depth cap, an over-range entry count, an over-range data Size, an out-of-range name-string offset, and a well-formed Type/Name/Language tree whose data round-trips byte-for-byte as a regression guard.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Hardens Win32 .rsrc resource-tree parsing in the decompiler against crafted or truncated PE inputs by adding explicit bounds checks, recursion limits, and cycle detection, and by introducing a resolver seam to bound resource data reads. This reduces the risk of uncatchable crashes (stack overflow / native OOB reads) when opening hostile assemblies (e.g., via “Save as project”).

Changes:

• Add a bounded resource-data resolver and carry section length through the directory walk to validate offsets/sizes.
• Clamp directory entry iteration, cap recursion depth, and break cycles during tree traversal.
• Add targeted unit tests that exercise the new guards using pinned crafted .rsrc buffers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
ICSharpCode.Decompiler/Util/Win32Resources.cs	Adds bounded parsing primitives (length tracking, in-bounds checks, depth/cycle guards) and refactors data access through a resolver delegate.
ICSharpCode.Decompiler.Tests/Util/Win32ResourcesTests.cs	Adds regression tests covering cycles, deep nesting, oversized counts/sizes, and valid-tree parsing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.