Bound WebCilFile section access against mapped-view length#3842
Open
christophwille wants to merge 1 commit into
Open
Bound WebCilFile section access against mapped-view length#3842christophwille wants to merge 1 commit into
christophwille wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens WebCilFile’s section/RVA handling to prevent out-of-bounds reads from crafted WebCIL/Wasm inputs, aligning the loader’s behavior more closely with the bounds-checked PE path and ensuring malformed inputs are rejected safely.
Changes:
- Adds mapped-view-length-aware bounds checking for section raw-data access and widens arithmetic to
longto avoid overflow/narrowing pitfalls. - Broadens structural robustness by catching key parsing exceptions in
FromFileand returningnull(the loader contract) rather than throwing. - Introduces a new
WebCilFileTestsfixture covering the bounds helper and end-to-end “return null, don’t throw” behavior for crafted containers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ICSharpCode.Decompiler/Metadata/WebCilFile.cs |
Adds view-length bounds checking and exception-to-null behavior for malformed/truncated WebCIL/Wasm inputs. |
ICSharpCode.Decompiler.Tests/Metadata/WebCilFileTests.cs |
Adds targeted unit tests for the new bounds helper and FromFile robustness on crafted inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WebCilFile builds raw native pointers into the memory-mapped view directly from section-header fields read out of attacker-controlled metadata. Unlike PEFile.GetSectionData, which delegates to the bounds-checked PEReader, this hand-rolled path validated nothing: a crafted section header could produce a SectionData (and hence a BlobReader) pointing far outside the view, an out-of-bounds read reachable on normal decompilation through method-body and field-data RVA resolution. The (int)RawDataSize narrowing cast could also yield a negative length. Resolve and bounds-check the raw-data range against the view length before constructing SectionData, widening the arithmetic to long so crafted uint fields cannot wrap the range check or narrow into an apparently valid length. Structural parsing in FromFile now reports a crafted or truncated module as "not a WebCIL file" (null) rather than letting EndOfStreamException, OverflowException or BadImageFormatException escape the loader. Assisted-by: Claude:claude-opus-4-8:Claude Code
5f610c5 to
a0f9f9d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
WebCilFile(the loader for WebCIL /.wasmmodules) resolves RVAs by building raw native pointers into the memory-mapped view directly from section-header fields read out of the module's metadata. UnlikePEFile.GetSectionData, which delegates to the bounds-checkedSystem.Reflection.Metadata.PEReader, this hand-rolled path validated nothing.A crafted section header (for example
VirtualAddress=0, VirtualSize=0xFFFFFFFF, RawDataPtr=0xFFFFFFFF, RawDataSize=0x7FFFFFFF) combined with an in-range RVA makesGetSectionDatareturn aSectionData(and therefore aBlobReader) pointing far outside the mapped view: an out-of-bounds read (CWE-125) that can surface as an access violation or as information disclosure. It is reachable during ordinary decompilation, since method-body resolution (GetMethodBody->GetSectionData(rva).GetReader()) and field-data resolution feed it RVAs taken straight from the metadata. The(int)RawDataSizenarrowing cast could additionally produce a negative length.The same file had related robustness gaps: a crafted or truncated module could drive the structural reads (the Wasm section loop, the data-segment loop, the WebCIL header, and RVA translation feeding
Seek/ReadInt32) past the end of the view, throwing an uncaughtEndOfStreamException/OverflowException/BadImageFormatExceptionout of the loader.Fix
GetSectionDatanow resolves the raw-data range through a single helper that bounds-checks it against the mapped-view length before constructingSectionData. All of the arithmetic is widened tolongso crafteduintheader fields cannot wrap the range check or narrow into a length that looks valid, and the length is rejected if it does not fit inint. The range checks inTranslateRVAandGetContainingSectionIndexare widened the same way.FromFilenow reports a crafted or truncated module as "not a WebCIL file" (returnsnull, the loader's existing contract) instead of letting those parsing exceptions escape.Tests
A new
WebCilFileTestsfixture covers the bounds helper directly (the exploit shape, an oversizedRawDataSize, a raw-data range running past the view, an RVA in no section, and a valid section that still resolves correctly) and exercisesFromFileend-to-end against crafted Wasm containers (truncated section-header table; a CLI-header RVA that falls in no section), asserting it returnsnullrather than throwing.🤖 Generated with Claude Code