Browse Source

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
pull/4047/head
Christoph Wille 3 weeks ago
parent
commit
bdb4ffaa56
  1. 253
      ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs
  2. 51
      ICSharpCode.Decompiler/Util/ResourcesFile.cs

253
ICSharpCode.Decompiler.Tests/Util/ResourcesFileTests.cs

@ -0,0 +1,253 @@ @@ -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
{
/// <summary>
/// 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.
/// </summary>
[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<BadImageFormatException>(() => 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<BadImageFormatException>(() => new ResourcesFile(new MemoryStream(blob)));
}
[Test]
public void ResourceNameLengthBeyondStream_ThrowsInsteadOfAllocating()
{
var blob = Build(DefaultReaderType, Array.Empty<string>(), new[] {
("name", StringPayload("value")),
}, forcedNameByteLength: int.MaxValue);
Assert.Throws<BadImageFormatException>(() => 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<string>(), new[] { ("bin", payload.ToArray()) });
Assert.Throws<BadImageFormatException>(() => 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<BadImageFormatException>(() => 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<BadImageFormatException>(() => 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();
}
/// <summary>
/// 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.
/// </summary>
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);
}
/// <summary>
/// Builds a complete version-2 .resources file. The data of each entry is the raw
/// data section record (type code followed by payload).
/// </summary>
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<int>();
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();
}
}
}

51
ICSharpCode.Decompiler/Util/ResourcesFile.cs

@ -152,18 +152,16 @@ namespace ICSharpCode.Decompiler.Util @@ -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 @@ -231,6 +229,22 @@ namespace ICSharpCode.Decompiler.Util
reader.Dispose();
}
/// <summary>
/// Validates a count or byte length read from the file before it is used to size an
/// allocation. Every element occupies at least <paramref name="bytesPerElement"/> 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.
/// </summary>
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 @@ -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 @@ -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 @@ -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);
}

Loading…
Cancel
Save