From bdb4ffaa56228ce95156e68bf41f9136bdfc39d1 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 25 Aug 2026 11:37:25 +0200 Subject: [PATCH] Bound every .resources length against the remaining stream before allocating A .resources file's resource count, type count, name lengths, binary resource lengths and serialized-object lengths all come from the file and were only checked for being non-negative before sizing an allocation. A crafted file can therefore request multi-gigabyte arrays from a few-hundred-byte payload (CWE-789), turning a click on a resource node into an out-of-memory condition. The serialization-format kind was additionally an assert-only check that vanishes in Release builds. Each element of these counts occupies at least one byte in the stream, so a value needing more bytes than remain after the current position cannot be honest. Reject it with the same BadImageFormatException the callers already handle, and promote the format-kind assert into a real check. Assisted-by: Claude:claude-fable-5:Claude Code --- .../Util/ResourcesFileTests.cs | 253 ++++++++++++++++++ ICSharpCode.Decompiler/Util/ResourcesFile.cs | 51 ++-- 2 files changed, 283 insertions(+), 21 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs diff --git a/ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs b/ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs new file mode 100644 index 000000000..5c02a578e --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs @@ -0,0 +1,253 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; + +using ICSharpCode.Decompiler.Util; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests.Util +{ + /// + /// Every count and length in a .resources file is attacker-controlled once a crafted + /// assembly is opened. These tests pin down that each one is validated against the data + /// that actually exists before it drives an allocation, so a malformed file fails with a + /// BadImageFormatException instead of an out-of-memory condition. + /// + [TestFixture] + public class ResourcesFileTests + { + const string DefaultReaderType = "System.Resources.ResourceReader, mscorlib"; + const string DeserializingReaderType = "System.Resources.Extensions.DeserializingResourceReader, System.Resources.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51"; + + // ResourceTypeCode values as written into the data section. + const int TypeCodeString = 1; + const int TypeCodeByteArray = 0x20; + const int TypeCodeStream = 33; + const int TypeCodeStartOfUserTypes = 0x40; + + [Test] + public void WellFormedFile_ReadsAllEntryKinds() + { + var blob = Build(DeserializingReaderType, new[] { "MyType, MyAssembly" }, new[] { + ("text", StringPayload("hello")), + ("blob", LengthPrefixedPayload(TypeCodeByteArray, new byte[] { 1, 2, 3 })), + ("stream", LengthPrefixedPayload(TypeCodeStream, new byte[] { 4, 5 })), + ("obj", SerializedObjectPayload(typeIndex: 0, kind: 2, new byte[] { 9, 8, 7 })), + }); + + var entries = new ResourcesFile(new MemoryStream(blob)).ToDictionary(e => e.Key, e => e.Value); + + Assert.That(entries["text"], Is.EqualTo("hello")); + Assert.That(entries["blob"], Is.EqualTo(new byte[] { 1, 2, 3 })); + Assert.That(((MemoryStream)entries["stream"]!).ToArray(), Is.EqualTo(new byte[] { 4, 5 })); + var obj = (ResourceSerializedObject)entries["obj"]!; + Assert.That(obj.TypeName, Is.EqualTo("MyType, MyAssembly")); + using var objStream = obj.GetStream(); + Assert.That(ReadAll(objStream), Is.EqualTo(new byte[] { 9, 8, 7 })); + } + + [Test] + public void TypeCountBeyondStream_ThrowsInsteadOfAllocating() + { + // int.MaxValue string references is a multi-gigabyte allocation request; the + // header only has room for a handful of type names. + var blob = BuildHeaderOnly(numResources: 0, numTypes: int.MaxValue); + + Assert.Throws(() => new ResourcesFile(new MemoryStream(blob))); + } + + [Test] + public void ResourceCountBeyondStream_ThrowsBeforeReadingPositions() + { + // Each resource needs at least a 4-byte name hash and a 4-byte name position, so a + // count larger than the remaining bytes can be rejected up front rather than after + // the position array has been allocated and the read has run off the end. + var blob = BuildHeaderOnly(numResources: 100_000, numTypes: 0); + + Assert.Throws(() => new ResourcesFile(new MemoryStream(blob))); + } + + [Test] + public void ResourceNameLengthBeyondStream_ThrowsInsteadOfAllocating() + { + var blob = Build(DefaultReaderType, Array.Empty(), new[] { + ("name", StringPayload("value")), + }, forcedNameByteLength: int.MaxValue); + + Assert.Throws(() => new ResourcesFile(new MemoryStream(blob)).ToList()); + } + + [TestCase(TypeCodeByteArray)] + [TestCase(TypeCodeStream)] + public void BinaryResourceLengthBeyondStream_ThrowsInsteadOfAllocating(int typeCode) + { + var payload = new MemoryStream(); + using (var w = new BinaryWriter(payload, Encoding.UTF8, leaveOpen: true)) + { + w.Write7BitEncodedInt(typeCode); + w.Write(int.MaxValue); // declared length, far beyond the file + w.Write(new byte[] { 1, 2, 3 }); + } + var blob = Build(DefaultReaderType, Array.Empty(), new[] { ("bin", payload.ToArray()) }); + + Assert.Throws(() => new ResourcesFile(new MemoryStream(blob)).ToList()); + } + + [Test] + public void SerializedObjectLengthBeyondStream_ThrowsInsteadOfAllocating() + { + var blob = Build(DeserializingReaderType, new[] { "MyType, MyAssembly" }, new[] { + ("obj", SerializedObjectPayload(typeIndex: 0, kind: 1, new byte[] { 1 }, forcedLength: int.MaxValue)), + }); + var obj = (ResourceSerializedObject)new ResourcesFile(new MemoryStream(blob)).Single().Value!; + + Assert.Throws(() => obj.GetStream()); + } + + [Test] + public void SerializedObjectWithUnknownFormatKind_Throws() + { + // Only the four SerializationFormat kinds defined by System.Resources.Extensions + // are valid; anything else means the data section is not what the header claims. + var blob = Build(DeserializingReaderType, new[] { "MyType, MyAssembly" }, new[] { + ("obj", SerializedObjectPayload(typeIndex: 0, kind: 99, new byte[] { 1 })), + }); + var obj = (ResourceSerializedObject)new ResourcesFile(new MemoryStream(blob)).Single().Value!; + + Assert.Throws(() => obj.GetStream()); + } + + static byte[] StringPayload(string value) + { + var ms = new MemoryStream(); + using (var w = new BinaryWriter(ms, Encoding.UTF8, leaveOpen: true)) + { + w.Write7BitEncodedInt(TypeCodeString); + w.Write(value); + } + return ms.ToArray(); + } + + static byte[] LengthPrefixedPayload(int typeCode, byte[] data) + { + var ms = new MemoryStream(); + using (var w = new BinaryWriter(ms, Encoding.UTF8, leaveOpen: true)) + { + w.Write7BitEncodedInt(typeCode); + w.Write(data.Length); + w.Write(data); + } + return ms.ToArray(); + } + + static byte[] SerializedObjectPayload(int typeIndex, int kind, byte[] data, int? forcedLength = null) + { + var ms = new MemoryStream(); + using (var w = new BinaryWriter(ms, Encoding.UTF8, leaveOpen: true)) + { + w.Write7BitEncodedInt(TypeCodeStartOfUserTypes + typeIndex); + w.Write7BitEncodedInt(kind); + w.Write7BitEncodedInt(forcedLength ?? data.Length); + w.Write(data); + } + return ms.ToArray(); + } + + static byte[] ReadAll(Stream stream) + { + var ms = new MemoryStream(); + stream.CopyTo(ms); + return ms.ToArray(); + } + + /// + /// Writes the ResourceManager and RuntimeResourceSet headers up to and including the + /// resource and type counts, then stops. The stream ends where the type names would be. + /// + static byte[] BuildHeaderOnly(int numResources, int numTypes) + { + var ms = new MemoryStream(); + using (var w = new BinaryWriter(ms, Encoding.UTF8, leaveOpen: true)) + { + WriteHeader(w, DefaultReaderType, numResources, numTypes); + } + return ms.ToArray(); + } + + static void WriteHeader(BinaryWriter w, string readerType, int numResources, int numTypes) + { + w.Write(ResourcesFile.MagicNumber); + w.Write(1); // ResourceManager header version + w.Write(0); // bytes to skip (unused for header version 1) + w.Write(readerType); + w.Write("System.Resources.RuntimeResourceSet"); + w.Write(2); // RuntimeResourceSet version + w.Write(numResources); + w.Write(numTypes); + } + + /// + /// Builds a complete version-2 .resources file. The data of each entry is the raw + /// data section record (type code followed by payload). + /// + static byte[] Build(string readerType, string[] typeNames, IList<(string Name, byte[] Data)> entries, int? forcedNameByteLength = null) + { + var nameSection = new MemoryStream(); + var dataSection = new MemoryStream(); + var namePositions = new List(); + using (var nameWriter = new BinaryWriter(nameSection, Encoding.UTF8, leaveOpen: true)) + { + foreach (var (name, data) in entries) + { + namePositions.Add((int)nameSection.Position); + byte[] nameBytes = Encoding.Unicode.GetBytes(name); + nameWriter.Write7BitEncodedInt(forcedNameByteLength ?? nameBytes.Length); + nameWriter.Write(nameBytes); + nameWriter.Write((int)dataSection.Position); + dataSection.Write(data); + } + } + + var ms = new MemoryStream(); + using (var w = new BinaryWriter(ms, Encoding.UTF8, leaveOpen: true)) + { + WriteHeader(w, readerType, entries.Count, typeNames.Length); + foreach (var typeName in typeNames) + w.Write(typeName); + while (ms.Position % 8 != 0) + w.Write((byte)0); + foreach (var (name, _) in entries) + w.Write(name.GetHashCode()); // name hashes are not consulted by ResourcesFile + foreach (int position in namePositions) + w.Write(position); + // name section starts right after this data section offset field + int nameSectionStart = (int)ms.Position + sizeof(int); + w.Write(nameSectionStart + (int)nameSection.Length); + w.Write(nameSection.ToArray()); + w.Write(dataSection.ToArray()); + } + return ms.ToArray(); + } + } +} diff --git a/ICSharpCode.Decompiler/Util/ResourcesFile.cs b/ICSharpCode.Decompiler/Util/ResourcesFile.cs index 4034b2d5a..8227f94b3 100644 --- a/ICSharpCode.Decompiler/Util/ResourcesFile.cs +++ b/ICSharpCode.Decompiler/Util/ResourcesFile.cs @@ -152,18 +152,16 @@ namespace ICSharpCode.Decompiler.Util throw new BadImageFormatException($"Unsupported resource set version: {version}"); numResources = reader.ReadInt32(); - if (numResources < 0) - { - throw new BadImageFormatException(ResourcesHeaderCorrupted); - } + // Every resource contributes at least a 4-byte name hash and a 4-byte name position. + // Bounding the count by the bytes that remain also keeps numResources * 2 in + // GetStartPositions from overflowing. + CheckLength(numResources, bytesPerElement: 8); // Read type positions into type positions array. // But delay initialize the type table. int numTypes = reader.ReadInt32(); - if (numTypes < 0) - { - throw new BadImageFormatException(ResourcesHeaderCorrupted); - } + // Every type name is a length-prefixed string of at least one byte. + CheckLength(numTypes, bytesPerElement: 1); typeTable = new string[numTypes]; for (int i = 0; i < numTypes; i++) { @@ -231,6 +229,22 @@ namespace ICSharpCode.Decompiler.Util reader.Dispose(); } + /// + /// Validates a count or byte length read from the file before it is used to size an + /// allocation. Every element occupies at least bytes + /// in the stream, so a value that needs more bytes than remain after the current + /// position cannot describe real data; rejecting it up front keeps a crafted header from + /// forcing a multi-gigabyte allocation. + /// + 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."); + } + } + public int ResourceCount => numResources; public string GetResourceName(int index) @@ -253,10 +267,7 @@ namespace ICSharpCode.Decompiler.Util reader.Seek(pos, SeekOrigin.Begin); // Can't use reader.ReadString, since it's using UTF-8! int byteLen = reader.Read7BitEncodedInt(); - if (byteLen < 0) - { - throw new BadImageFormatException("Resource name has negative length"); - } + CheckLength(byteLen, bytesPerElement: 1); bytes = new byte[byteLen]; // We must read byteLen bytes, or we have a corrupted file. // Use a blocking read in case the stream doesn't give us back @@ -448,20 +459,14 @@ namespace ICSharpCode.Decompiler.Util case ResourceTypeCode.ByteArray: { int len = reader.ReadInt32(); - if (len < 0) - { - throw new BadImageFormatException("Resource with negative length"); - } + CheckLength(len, bytesPerElement: 1); return reader.ReadBytes(len); } case ResourceTypeCode.Stream: { int len = reader.ReadInt32(); - if (len < 0) - { - throw new BadImageFormatException("Resource with negative length"); - } + CheckLength(len, bytesPerElement: 1); byte[] bytes = reader.ReadBytes(len); return new MemoryStream(bytes, writable: false); } @@ -548,8 +553,12 @@ namespace ICSharpCode.Decompiler.Util if (usesSerializationFormat) { int kind = reader.Read7BitEncodedInt(); - Debug.Assert(Enum.IsDefined(typeof(SerializationFormat), kind)); + if (!Enum.IsDefined(typeof(SerializationFormat), kind)) + { + throw new BadImageFormatException("Resources file corrupted: unknown serialization format."); + } len = reader.Read7BitEncodedInt(); + CheckLength(len, bytesPerElement: 1); } return reader.ReadBytes(len); }