Harden BAML reader against crafted-resource crashes

[christophwille]

Browsing an assembly's embedded .baml resource runs it through BamlReader.ReadDocument, whose post-parse "defer" pass walks the record list with NavigateTree. That walk had two defects, both reachable from ordinary resource browsing (no "save as project" / project export required): it recursed on every nested StaticResourceStart / KeyElementStart record with no depth cap, and it indexed the record list (doc[index], a List<BamlRecord>) with no bound.

A crafted resource with tens of thousands of nested start records (~4 bytes each) drives the recursion into a StackOverflowException. That exception is uncatchable in .NET and terminates the ILSpy process even though BamlResourceEntryNode.Decompile wraps the call in try/catch — the catch cannot see a stack overflow. A start record with no matching end instead walks the index past the end of the list. The project-export path (BamlResourceFileHandler.WriteResourceToFile) has no catch at all, so there even the index overrun is unhandled.

This consolidates the (previously duplicated) defer walk into a single helper that bounds every record-list access and depth-caps the nesting walk, failing with a catchable InvalidDataException. The existing UI catch then turns a malformed resource into a "BAML decompilation failed" message instead of a crash. The cap (1000) sits far below the CLR stack limit, so it never rejects legitimate BAML, which nests only a handful of levels.

Two smaller hardening fixes in the same reader, same root cause of trusting attacker-controlled offsets/lengths: ReadSignature now rejects a signature length that cannot fit in the remaining stream before it allocates (the length is read before the MSBAML check, so a crafted value otherwise drives a multi-gigabyte allocation), and the defer pass resolves record offsets with TryGetValue so a bogus offset reports malformed data rather than escaping as a bare KeyNotFoundException.

Tests in ILSpy.BamlDecompiler.Tests craft minimal malformed .baml streams and assert that the public XamlDecompiler.Decompile entry point throws InvalidDataException for the deeply-nested, unterminated, and oversized-signature cases rather than crashing or allocating. They live in the existing WPF-bound (Windows-only) BAML test project alongside the other cases.

[Copilot]

Pull request overview

Hardens the BAML decompiler’s reader/defer-pass against crafted .baml resources that previously could crash ILSpy (uncatchable StackOverflowException, out-of-range indexing, oversized allocations) by adding bounded record navigation, safer offset resolution, and targeted regression tests.

Changes:

• Introduces BamlDeferReader to centralize and harden defer-block key scanning with bounded indexing and a maximum nesting depth.
• Uses TryGetValue when resolving defer-record offsets so malformed offsets surface as InvalidDataException instead of KeyNotFoundException.
• Adds Windows/WPF-bound tests that craft malformed BAML streams to assert InvalidDataException behavior (deep nesting, unterminated blocks, oversized signature length).

Reviewed changes

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

File	Description
ILSpy.BamlDecompiler.Tests/ILSpy.BamlDecompiler.Tests.csproj	Includes new security regression test file in the test project build.
ILSpy.BamlDecompiler.Tests/BamlSecurityTests.cs	Adds crafted-stream regression tests for previously crashable/memory-intensive BAML inputs.
ICSharpCode.BamlDecompiler/Baml/BamlRecords.cs	Adds BamlDeferReader helper and replaces duplicated defer key-walk logic with bounded/depth-capped traversal.
ICSharpCode.BamlDecompiler/Baml/BamlReader.cs	Adds signature-length validation and makes defer offset resolution robust via TryGetValue + InvalidDataException.

---

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