Bound every .resources length against the remaining stream before allocating

[christophwille]

Summary

Validates every count and length that a .resources file supplies against the bytes that actually remain in the stream before that value is used to size an allocation. A crafted resource can no longer turn a few hundred bytes into a multi-gigabyte allocation request, and the serialization-format kind check is now a real validation instead of a Debug.Assert.

The vulnerability (CWE-789, allocation with excessive size from untrusted input)

ICSharpCode.Decompiler/Util/ResourcesFile.cs is a hardened port of the .NET ResourceReader. The residual exposure is allocation-size denial of service: several values are read from the file, checked only for being non-negative, and then immediately drive an allocation.

Site	Untrusted value	Allocation
constructor	numTypes (int32)	new string[numTypes] - up to 16 GB of references
constructor	numResources (int32)	new int[numResources] - up to 2 GB (after the existing checked(4 * numResources) seek caps it at int.MaxValue / 4)
GetResourceName	name byteLen (7-bit int)	new byte[byteLen] - up to 2 GB
LoadObjectV2 ByteArray / Stream	len (int32)	ReadBytes(len) - up to 2 GB
GetBytesForSerializedObject	len (7-bit int) when the reader type is DeserializingResourceReader	ReadBytes(len) - up to 2 GB

The last row also relied on Debug.Assert(Enum.IsDefined(typeof(SerializationFormat), kind)), which is compiled out of Release builds, so an unknown kind was silently accepted.

Values above the CLR's maximum array length fail immediately with OutOfMemoryException ("Array dimensions exceeded supported range"); values just below it commit real memory before the subsequent read fails. Either way the failure is an OOM, not the BadImageFormatException the callers (ResourcesFileTreeNode, WholeProjectDecompiler, ilspycmd) are written to catch.

Reachability

Expanding a .resources node in the resource tree (ResourcesFileTreeNode.LoadChildren -> new ResourcesFile(stream)), exporting to ResX, whole-project decompilation, and ilspycmd resource extraction all parse the file this way. No action beyond opening the assembly and clicking the resource is needed.

What was re-verified and found not to be a problem

• numResources * 2 in GetStartPositions cannot overflow. The constructor already performs reader.Seek(checked(4 * numResources), ...) and converts the OverflowException into a BadImageFormatException, so any ResourcesFile instance that exists has numResources <= int.MaxValue / 4, and numResources * 2 <= int.MaxValue / 2. The new bound (numResources * 8 <= remaining bytes) tightens this further; no change to GetStartPositions was needed.
• BinaryReader.ReadString (type names, string resources) is already safe. It reads in 128-byte chunks into a StringBuilder rather than pre-allocating the declared length, so a huge declared length just hits end-of-stream.

The fix

One private helper, applied at every site in the table:

void CheckLength(int count, int bytesPerElement)
{
    long remaining = reader.BaseStream.Length - reader.BaseStream.Position;
    if (count < 0 || (long)count * bytesPerElement > remaining)
        throw new BadImageFormatException("Resources file corrupted: declared length exceeds the available data.");
}

Every element of these counts occupies at least bytesPerElement bytes in the stream (8 per resource: a 4-byte name hash plus a 4-byte name position; 1 per type name, since each is a length-prefixed string; 1 per byte of any byte length). A value that would need more bytes than remain cannot describe real data, so it is rejected before any allocation happens. The bound is relative to the input, so it never rejects a well-formed file of any size and introduces no arbitrary constant.

ResourcesFile documents that its stream must be seekable, so Length/Position are always available.

The Debug.Assert on the serialization-format kind is promoted to a BadImageFormatException, since only the four kinds defined by System.Resources.Extensions are valid.

Tests

ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs (new), with a small in-test .resources writer:

• Well-formed file with a string, byte array, stream and DeserializingResourceReader serialized object round-trips (guards against the bounds rejecting legitimate data).
• numTypes = int.MaxValue -> BadImageFormatException (was OutOfMemoryException).
• numResources = 100000 on a header-only stream -> BadImageFormatException before positions are read (was EndOfStreamException after allocating).
• Name length int.MaxValue -> BadImageFormatException (was OutOfMemoryException).
• ByteArray and Stream length int.MaxValue -> BadImageFormatException (was OutOfMemoryException).
• Serialized-object length int.MaxValue -> BadImageFormatException (was OutOfMemoryException).
• Unknown serialization-format kind -> BadImageFormatException (was an assertion failure in Debug, silently accepted in Release).

All seven failure cases were confirmed red before the change and green after; the existing AvaloniaResourcesFileTests still pass.

---

Generated with Claude Code

[siegfriedpammer]

Call it CheckLengthOrThrow?

Reviewed with Stampeded!

[christophwille]

Done in 629bed5.