diff --git a/ICSharpCode.Decompiler.Tests/Util/Win32ResourcesTests.cs b/ICSharpCode.Decompiler.Tests/Util/Win32ResourcesTests.cs new file mode 100644 index 000000000..9c7a66908 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/Util/Win32ResourcesTests.cs @@ -0,0 +1,256 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// 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.Runtime.InteropServices; + +using ICSharpCode.Decompiler.Util; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests.Util +{ + // Exercises the bounds, recursion-depth and cycle guards in Win32Resources against crafted + // .rsrc section bytes. Each test hands a hand-built directory tree to the parser through a + // resolver that maps an RVA to an offset inside the same pinned buffer (a single-section PE). + [TestFixture] + public unsafe class Win32ResourcesTests + { + const int DirectorySize = 16; // IMAGE_RESOURCE_DIRECTORY + const int EntrySize = 8; // IMAGE_RESOURCE_DIRECTORY_ENTRY + const int DataEntrySize = 16; // IMAGE_RESOURCE_DATA_ENTRY + const uint SubdirectoryFlag = 0x80000000; + + // Pins the buffer, parses it as a resource section, and runs the assertions while the data + // pointers captured during parsing still point into the pinned buffer. + static void Parse(byte[] buffer, Action assert) + { + var handle = GCHandle.Alloc(buffer, GCHandleType.Pinned); + try + { + byte* pRoot = (byte*)handle.AddrOfPinnedObject(); + var resolver = new BufferResolver(pRoot, buffer.Length); + var root = Win32ResourceDirectory.ReadDirectoryTree(pRoot, buffer.Length, resolver.Resolve); + assert(root); + } + finally + { + handle.Free(); + } + } + + // Resolves a data RVA to a pointer inside the buffer, returning the bytes that remain from + // that offset to the end - the same "length to end of section" contract PEReader.GetSectionData + // provides, so a crafted Size larger than the data can be bounded. + sealed class BufferResolver + { + readonly byte* pRoot; + readonly int length; + + public BufferResolver(byte* pRoot, int length) + { + this.pRoot = pRoot; + this.length = length; + } + + public byte* Resolve(int rva, out int dataLength) + { + if (rva < 0 || rva > length) + { + dataLength = 0; + return null; + } + dataLength = length - rva; + return pRoot + rva; + } + } + + static void WriteDirectory(byte[] buffer, int offset, ushort namedEntries, ushort idEntries) + { + BitConverter.GetBytes(namedEntries).CopyTo(buffer, offset + 12); + BitConverter.GetBytes(idEntries).CopyTo(buffer, offset + 14); + } + + static void WriteEntry(byte[] buffer, int offset, uint name, uint offsetToData) + { + BitConverter.GetBytes(name).CopyTo(buffer, offset); + BitConverter.GetBytes(offsetToData).CopyTo(buffer, offset + 4); + } + + static void WriteDataEntry(byte[] buffer, int offset, uint rva, uint size) + { + BitConverter.GetBytes(rva).CopyTo(buffer, offset); + BitConverter.GetBytes(size).CopyTo(buffer, offset + 4); + } + + [Test] + public void SelfReferentialSubdirectory_DoesNotRecurseInfinitely() + { + // One directory with a single subdirectory entry that points back at itself (offset 0). + // The unfixed parser follows it forever, yielding an uncatchable StackOverflowException. + byte[] buffer = new byte[DirectorySize + EntrySize]; + WriteDirectory(buffer, 0, namedEntries: 0, idEntries: 1); + WriteEntry(buffer, DirectorySize, name: 1, offsetToData: SubdirectoryFlag /* offset 0 */); + + Parse(buffer, root => { + Assert.That(root.Directories.Count, Is.EqualTo(1)); + var child = root.Directories[0]; + Assert.That(child.Directories.Count, Is.EqualTo(0), "the cycle back to the root must be cut"); + Assert.That(child.Datas.Count, Is.EqualTo(0)); + }); + } + + [Test] + public void DeeplyNestedDirectories_AreBoundedByDepthLimit() + { + // A long chain of distinct nested directories. Even without a cycle this would recurse + // as deep as the chain; the depth cap must stop it well before that. + const int chainLength = 40; + byte[] buffer = new byte[chainLength * (DirectorySize + EntrySize)]; + for (int k = 0; k < chainLength; k++) + { + int dirOffset = k * (DirectorySize + EntrySize); + bool hasChild = k < chainLength - 1; + WriteDirectory(buffer, dirOffset, namedEntries: 0, idEntries: (ushort)(hasChild ? 1 : 0)); + if (hasChild) + { + uint childOffset = (uint)((k + 1) * (DirectorySize + EntrySize)); + WriteEntry(buffer, dirOffset + DirectorySize, name: (uint)(k + 1), offsetToData: SubdirectoryFlag | childOffset); + } + } + + Parse(buffer, root => { + int depth = 0; + var current = root; + while (current != null && current.Directories.Count > 0) + { + current = current.Directories[0]; + depth++; + } + // The parser caps nesting at a small constant (well above any real resource tree), + // so the measured depth must be far below the crafted chain length. + Assert.That(depth, Is.LessThanOrEqualTo(17)); + }); + } + + [Test] + public void EntryCountBeyondSection_IsClamped() + { + // A directory header that claims far more entries than the section can hold. The unfixed + // parser walks the declared count straight off the end of the section. + byte[] buffer = new byte[DirectorySize]; + WriteDirectory(buffer, 0, namedEntries: 0, idEntries: 0xFFFF); + + Parse(buffer, root => { + Assert.That(root.Directories.Count, Is.EqualTo(0)); + Assert.That(root.Datas.Count, Is.EqualTo(0)); + }); + } + + [Test] + public void DataSizeBeyondSection_IsClampedToAvailable() + { + // A data leaf whose declared Size dwarfs the bytes actually present. The unfixed Data + // getter copies the full Size, reading gigabytes past the section base. + const int dataBytes = 8; + int dataEntryOffset = DirectorySize + EntrySize; + int dataOffset = dataEntryOffset + DataEntrySize; + byte[] buffer = new byte[dataOffset + dataBytes]; + WriteDirectory(buffer, 0, namedEntries: 0, idEntries: 1); + WriteEntry(buffer, DirectorySize, name: 1, offsetToData: (uint)dataEntryOffset /* data leaf */); + WriteDataEntry(buffer, dataEntryOffset, rva: (uint)dataOffset, size: 0xFFFFFFF0); + + Parse(buffer, root => { + Assert.That(root.Datas.Count, Is.EqualTo(1)); + var data = root.Datas[0]; + Assert.That(data.Size, Is.EqualTo(0xFFFFFFF0)); + Assert.That(data.Data.Length, Is.EqualTo(dataBytes), "the copy must be bounded to the bytes that exist"); + }); + } + + [Test] + public void NegativeDataRva_YieldsEmptyDataWithoutThrowing() + { + // The data entry's RVA is a file uint; with the high bit set it casts to a negative int. + // The resolver must reject it (PEReader.GetSectionData throws on a negative RVA) so the + // leaf yields empty data rather than aborting the parse. + int dataEntryOffset = DirectorySize + EntrySize; + byte[] buffer = new byte[dataEntryOffset + DataEntrySize]; + WriteDirectory(buffer, 0, namedEntries: 0, idEntries: 1); + WriteEntry(buffer, DirectorySize, name: 1, offsetToData: (uint)dataEntryOffset /* data leaf */); + WriteDataEntry(buffer, dataEntryOffset, rva: 0xFFFFFFFF /* negative as int */, size: 0x100); + + Parse(buffer, root => { + Assert.That(root.Datas.Count, Is.EqualTo(1)); + Assert.That(root.Datas[0].Data, Is.Empty); + }); + } + + [Test] + public void OutOfRangeStringName_DoesNotReadOutOfBounds() + { + // A named entry whose name-string offset lies past the section end. The unfixed parser + // dereferences it directly, reading the length prefix and characters out of bounds. + int dataEntryOffset = DirectorySize + EntrySize; + byte[] buffer = new byte[dataEntryOffset + DataEntrySize]; + WriteDirectory(buffer, 0, namedEntries: 1, idEntries: 0); + WriteEntry(buffer, DirectorySize, name: SubdirectoryFlag | 0x100 /* string offset past the buffer */, offsetToData: (uint)dataEntryOffset); + WriteDataEntry(buffer, dataEntryOffset, rva: 0, size: 0); + + Parse(buffer, root => { + Assert.That(root.Datas.Count, Is.EqualTo(1)); + var name = root.Datas[0].Name; + Assert.That(name.HasName, Is.True); + Assert.That(name.Name, Is.Empty, "an out-of-range string name must resolve to empty, not an OOB read"); + }); + } + + [Test] + public void ValidResourceTree_ParsesAndReadsData() + { + // A well-formed Type -> Name -> (language data leaf) tree, mirroring how a manifest is + // laid out, to prove the bounds checks do not break normal parsing. + const int RT_MANIFEST = 24; + int typeDir = 0; + int rootEntry = typeDir + DirectorySize; // 16 + int nameDir = rootEntry + EntrySize; // 24 + int typeEntry = nameDir + DirectorySize; // 40 + int leafDir = typeEntry + EntrySize; // 48 + int nameEntry = leafDir + DirectorySize; // 64 + int dataEntry = nameEntry + EntrySize; // 72 + int dataOffset = dataEntry + DataEntrySize; // 88 + byte[] payload = { 0xDE, 0xAD, 0xBE, 0xEF }; + byte[] buffer = new byte[dataOffset + payload.Length]; + + WriteDirectory(buffer, typeDir, namedEntries: 0, idEntries: 1); + WriteEntry(buffer, rootEntry, name: RT_MANIFEST, offsetToData: SubdirectoryFlag | (uint)nameDir); + WriteDirectory(buffer, nameDir, namedEntries: 0, idEntries: 1); + WriteEntry(buffer, typeEntry, name: 1, offsetToData: SubdirectoryFlag | (uint)leafDir); + WriteDirectory(buffer, leafDir, namedEntries: 0, idEntries: 1); + WriteEntry(buffer, nameEntry, name: 1033, offsetToData: (uint)dataEntry /* data leaf */); + WriteDataEntry(buffer, dataEntry, rva: (uint)dataOffset, size: (uint)payload.Length); + payload.CopyTo(buffer, dataOffset); + + Parse(buffer, root => { + var manifest = root.Find(new Win32ResourceName(RT_MANIFEST))?.FirstDirectory()?.FirstData()?.Data; + Assert.That(manifest, Is.Not.Null); + Assert.That(manifest, Is.EqualTo(payload)); + }); + } + } +} diff --git a/ICSharpCode.Decompiler/Util/Win32Resources.cs b/ICSharpCode.Decompiler/Util/Win32Resources.cs index e2a00d525..84721ded1 100644 --- a/ICSharpCode.Decompiler/Util/Win32Resources.cs +++ b/ICSharpCode.Decompiler/Util/Win32Resources.cs @@ -6,6 +6,13 @@ using System.Reflection.PortableExecutable; namespace ICSharpCode.Decompiler.Util { + /// + /// Resolves a resource data RVA to a pointer into the PE image and reports how many bytes remain + /// from that point to the end of the containing section, so a crafted Size can be bounded against + /// the data that actually exists. + /// + internal unsafe delegate byte* ResolveResourceData(int rva, out int length); + /// /// Represents win32 resources /// @@ -24,10 +31,39 @@ namespace ICSharpCode.Decompiler.Util } int rva = pe.PEHeaders.PEHeader?.ResourceTableDirectory.RelativeVirtualAddress ?? 0; - if (rva == 0) + // A crafted header can set the directory RVA negative; GetSectionData throws on a negative + // RVA, so reject it here rather than let it abort parsing. + if (rva <= 0) + return null; + var block = pe.GetSectionData(rva); + if (block.Pointer == null || block.Length <= 0) return null; - byte* pRoot = pe.GetSectionData(rva).Pointer; - return new Win32ResourceDirectory(pe, pRoot, 0, new Win32ResourceName("Root")); + + byte* Resolve(int dataRva, out int length) + { + // OffsetToData is a file uint cast to int, so it can be negative; GetSectionData throws + // on a negative RVA, and a positive RVA outside any section yields an empty block. + if (dataRva < 0) + { + length = 0; + return null; + } + var dataBlock = pe.GetSectionData(dataRva); + length = dataBlock.Length; + return dataBlock.Pointer; + } + + return Win32ResourceDirectory.ReadDirectoryTree(block.Pointer, block.Length, Resolve); + } + + /// + /// Returns true when reading bytes at stays + /// within a section of bytes. Subtraction avoids the overflow an + /// offset + size bound would have on attacker-controlled values. + /// + internal static bool InBounds(int offset, int size, int length) + { + return offset >= 0 && size >= 0 && offset <= length && size <= length - offset; } public static Win32ResourceDirectory? Find(this Win32ResourceDirectory root, Win32ResourceName type) @@ -91,8 +127,33 @@ namespace ICSharpCode.Decompiler.Util public IList Datas { get; } - internal unsafe Win32ResourceDirectory(PEReader pe, byte* pRoot, int offset, Win32ResourceName name) + // Windows resource trees are conventionally Type -> Name -> Language (three directory levels). + // This cap sits well above that, so it never rejects a real tree, while keeping a crafted deep + // chain from exhausting the stack - which the cycle check alone cannot, since a non-repeating + // chain of distinct offsets is acyclic yet still arbitrarily deep. + const int MaxDepth = 16; + + internal static unsafe Win32ResourceDirectory ReadDirectoryTree(byte* pRoot, int length, ResolveResourceData resolveData) { + return new Win32ResourceDirectory(resolveData, pRoot, length, 0, new Win32ResourceName("Root"), 0, new HashSet()); + } + + unsafe Win32ResourceDirectory(ResolveResourceData resolveData, byte* pRoot, int length, int offset, Win32ResourceName name, int depth, HashSet visited) + { + Name = name; + Directories = new List(); + Datas = new List(); + + // A crafted .rsrc tree can be cyclic or arbitrarily deep. The depth cap bounds nesting; the + // visited set, shared across the whole walk, parses each directory offset at most once, + // which breaks cycles and caps total work at the distinct offsets in the section. Real + // resource trees never reference one directory from two branches, so parsing a repeated + // offset only once is lossless for valid input. + if (depth > MaxDepth || !visited.Add(offset)) + return; + if (!Win32Resources.InBounds(offset, sizeof(IMAGE_RESOURCE_DIRECTORY), length)) + return; + var p = (IMAGE_RESOURCE_DIRECTORY*)(pRoot + offset); Characteristics = p->Characteristics; TimeDateStamp = p->TimeDateStamp; @@ -101,28 +162,28 @@ namespace ICSharpCode.Decompiler.Util NumberOfNamedEntries = p->NumberOfNamedEntries; NumberOfIdEntries = p->NumberOfIdEntries; - Name = name; - Directories = new List(); - Datas = new List(); - var pEntries = (IMAGE_RESOURCE_DIRECTORY_ENTRY*)(p + 1); + int entriesOffset = offset + sizeof(IMAGE_RESOURCE_DIRECTORY); int total = NumberOfNamedEntries + NumberOfIdEntries; + // Clamp the file-declared entry count to the entries that actually fit before the section + // end, so the walk below cannot read past it. + int available = (length - entriesOffset) / sizeof(IMAGE_RESOURCE_DIRECTORY_ENTRY); + if (available < 0) + available = 0; + if (total > available) + total = available; + + var pEntries = (IMAGE_RESOURCE_DIRECTORY_ENTRY*)(pRoot + entriesOffset); for (int i = 0; i < total; i++) { var pEntry = pEntries + i; - name = new Win32ResourceName(pRoot, pEntry); + name = new Win32ResourceName(pRoot, length, pEntry); if ((pEntry->OffsetToData & 0x80000000) == 0) - Datas.Add(new Win32ResourceData(pe, pRoot, (int)pEntry->OffsetToData, name)); + Datas.Add(new Win32ResourceData(resolveData, pRoot, length, (int)pEntry->OffsetToData, name)); else - Directories.Add(new Win32ResourceDirectory(pe, pRoot, (int)(pEntry->OffsetToData & 0x7FFFFFFF), name)); + Directories.Add(new Win32ResourceDirectory(resolveData, pRoot, length, (int)(pEntry->OffsetToData & 0x7FFFFFFF), name, depth + 1, visited)); } } - static unsafe string ReadString(byte* pRoot, int offset) - { - var pString = (IMAGE_RESOURCE_DIRECTORY_STRING*)(pRoot + offset); - return new string(pString->NameString, 0, pString->Length); - } - public Win32ResourceDirectory? FindDirectory(Win32ResourceName name) { foreach (var directory in Directories) @@ -164,29 +225,41 @@ namespace ICSharpCode.Decompiler.Util public uint Reserved { get; } #endregion - private readonly void* _pointer; + private readonly byte* _pointer; + private readonly int _dataLength; public Win32ResourceName Name { get; } public byte[] Data { get { - byte[] data = new byte[Size]; + // Size is file-controlled (up to 4 GB); bound both the allocation and the copy to the + // bytes the resolver reports between the data pointer and the end of its section. That + // length is never negative, so the clamped count fits in an int. + if (_pointer == null || _dataLength <= 0) + return Array.Empty(); + int count = (int)Math.Min((uint)Size, (uint)_dataLength); + byte[] data = new byte[count]; fixed (void* pData = data) - Buffer.MemoryCopy(_pointer, pData, Size, Size); + Buffer.MemoryCopy(_pointer, pData, count, count); return data; } } - internal Win32ResourceData(PEReader pe, byte* pRoot, int offset, Win32ResourceName name) + internal Win32ResourceData(ResolveResourceData resolveData, byte* pRoot, int length, int offset, Win32ResourceName name) { + Name = name; + if (!Win32Resources.InBounds(offset, sizeof(IMAGE_RESOURCE_DATA_ENTRY), length)) + return; + var p = (IMAGE_RESOURCE_DATA_ENTRY*)(pRoot + offset); OffsetToData = p->OffsetToData; Size = p->Size; CodePage = p->CodePage; Reserved = p->Reserved; - _pointer = pe.GetSectionData((int)OffsetToData).Pointer; - Name = name; + // OffsetToData is an RVA that can exceed int range; wrap deliberately (the library builds + // checked) so an out-of-range value reaches the resolver as a negative RVA it rejects. + _pointer = resolveData(unchecked((int)OffsetToData), out _dataLength); } } @@ -216,14 +289,22 @@ namespace ICSharpCode.Decompiler.Util _name = id; } - internal unsafe Win32ResourceName(byte* pRoot, IMAGE_RESOURCE_DIRECTORY_ENTRY* pEntry) + internal unsafe Win32ResourceName(byte* pRoot, int length, IMAGE_RESOURCE_DIRECTORY_ENTRY* pEntry) { - _name = (pEntry->Name & 0x80000000) == 0 ? (object)(ushort)pEntry->Name : ReadString(pRoot, (int)(pEntry->Name & 0x7FFFFFFF)); + _name = (pEntry->Name & 0x80000000) == 0 ? (object)(ushort)pEntry->Name : ReadString(pRoot, length, (int)(pEntry->Name & 0x7FFFFFFF)); - static string ReadString(byte* pRoot, int offset) + static string ReadString(byte* pRoot, int length, int offset) { + // A ushort character count followed by that many UTF-16 chars. Reject a prefix that + // runs past the section and clamp the character count to the bytes that remain. + if (!Win32Resources.InBounds(offset, sizeof(ushort), length)) + return string.Empty; var pString = (IMAGE_RESOURCE_DIRECTORY_STRING*)(pRoot + offset); - return new string(pString->NameString, 0, pString->Length); + int charCount = pString->Length; + int available = (length - (offset + sizeof(ushort))) / sizeof(char); + if (charCount > available) + charCount = available; + return new string(pString->NameString, 0, charCount); } }