Harden BAML reader against crafted-resource crashes

Browsing an embedded .baml resource runs it through BamlReader.ReadDocument,
whose post-parse defer pass walks the record list with NavigateTree. That walk
recursed on every nested StaticResourceStart/KeyElementStart record with no
depth cap and indexed the record list with no bound. A crafted resource could
therefore drive recursion into a StackOverflowException -- which is uncatchable
and kills the process despite the resource node's try/catch -- or walk the index
off the end of the list. Both are reachable from ordinary resource browsing, no
project export required.

Consolidate the duplicated defer walk into one bounded, depth-capped helper that
fails with a catchable InvalidDataException, so the existing UI catch turns a
malformed resource into a "BAML decompilation failed" message instead of a crash.
Reject an oversized signature length before it drives a multi-gigabyte allocation
(it is read before the MSBAML check), and resolve defer offsets with TryGetValue
so a bogus offset reports malformed data rather than escaping as a bare
KeyNotFoundException.

Assisted-by: Claude:claude-opus-4-8:Claude Code

ICSharpCode.BamlDecompiler/Baml/BamlReader.cs

@@ -64,6 +64,11 @@ namespace ICSharpCode.BamlDecompiler.Baml
 		{
 			var rdr = new BinaryReader(str, Encoding.Unicode);
 			uint len = rdr.ReadUInt32();
+			// len is read straight from the file, before the MSBAML_SIG check below. The only
+			// accepted signature is the fixed-length "MSBAML", so reject any other length here
+			// rather than allocating an attacker-sized string from a crafted value.
+			if (len >> 1 != (uint)MSBAML_SIG.Length)
+				throw new InvalidDataException("Invalid BAML signature length.");
 			var sig = new string(rdr.ReadChars((int)(len >> 1)));
 			rdr.ReadBytes((int)(((len + 3) & ~3) - len));
 			return sig;
@@ -263,7 +268,9 @@ namespace ICSharpCode.BamlDecompiler.Baml
 			for (int i = 0; i < ret.Count; i++)
 			{
 				if (ret[i] is IBamlDeferRecord defer)
-					defer.ReadDefer(ret, i, _ => recs[_]);
+					defer.ReadDefer(ret, i, offset => recs.TryGetValue(offset, out var rec)
+						? rec
+						: throw new InvalidDataException("BAML defer record points at an offset that is not a record boundary."));
 			}
 
 			return ret;

ICSharpCode.BamlDecompiler/Baml/BamlRecords.cs

@@ -149,6 +149,79 @@ namespace ICSharpCode.BamlDecompiler.Baml
 		void WriteDefer(BamlDocument doc, int index, BinaryWriter wtr);
 	}
 
+	/// <summary>
+	/// Walks the key records that precede a defer target. The record offsets are part of
+	/// the BAML payload and therefore attacker-controlled, so every list access is bounded
+	/// and the nesting walk is depth-capped: a crafted resource fails with a catchable
+	/// <see cref="InvalidDataException"/> instead of reading past the end of the record list
+	/// or recursing until the process dies with an uncatchable StackOverflowException.
+	/// </summary>
+	internal static class BamlDeferReader
+	{
+		// Legitimate BAML defer blocks nest only a handful of levels; this cap sits far
+		// below the CLR stack limit, so it never rejects real input but stops a crafted
+		// chain of nested start records before it overflows the stack.
+		const int MaxNestingDepth = 1000;
+
+		static InvalidDataException Malformed(string detail)
+			=> new InvalidDataException("Malformed BAML defer block: " + detail + ".");
+
+		/// <summary>
+		/// Advances past the leading key records of a defer block and returns the index of
+		/// the record that terminates it.
+		/// </summary>
+		public static int SkipKeys(BamlDocument doc, int index)
+		{
+			bool keys = true;
+			do
+			{
+				if (index < 0 || index >= doc.Count)
+					throw Malformed("ran off the end of the record list while scanning keys");
+				switch (doc[index].Type)
+				{
+					case BamlRecordType.DefAttributeKeyString:
+					case BamlRecordType.DefAttributeKeyType:
+					case BamlRecordType.OptimizedStaticResource:
+						keys = true;
+						break;
+					case BamlRecordType.StaticResourceStart:
+						NavigateTree(doc, BamlRecordType.StaticResourceStart, BamlRecordType.StaticResourceEnd, ref index, 0);
+						keys = true;
+						break;
+					case BamlRecordType.KeyElementStart:
+						NavigateTree(doc, BamlRecordType.KeyElementStart, BamlRecordType.KeyElementEnd, ref index, 0);
+						keys = true;
+						break;
+					default:
+						keys = false;
+						index--;
+						break;
+				}
+				index++;
+			} while (keys);
+			if (index < 0 || index >= doc.Count)
+				throw Malformed("ran off the end of the record list before the defer target");
+			return index;
+		}
+
+		static void NavigateTree(BamlDocument doc, BamlRecordType start, BamlRecordType end, ref int index, int depth)
+		{
+			if (depth >= MaxNestingDepth)
+				throw Malformed("nested start records exceed the maximum supported depth");
+			index++;
+			while (true)
+			{
+				if (index >= doc.Count)
+					throw Malformed("a start record has no matching end record");
+				if (doc[index].Type == start)
+					NavigateTree(doc, start, end, ref index, depth + 1);
+				else if (doc[index].Type == end)
+					return;
+				index++;
+			}
+		}
+	}
+
 	internal class XmlnsPropertyRecord : SizedBamlRecord
 	{
 		public override BamlRecordType Type => BamlRecordType.XmlnsProperty;
@@ -336,61 +409,13 @@ namespace ICSharpCode.BamlDecompiler.Baml
 
 		public void ReadDefer(BamlDocument doc, int index, Func<long, BamlRecord> resolve)
 		{
-			bool keys = true;
+			index = BamlDeferReader.SkipKeys(doc, index);
-			do
-			{
-				switch (doc[index].Type)
-				{
-					case BamlRecordType.DefAttributeKeyString:
-					case BamlRecordType.DefAttributeKeyType:
-					case BamlRecordType.OptimizedStaticResource:
-						keys = true;
-						break;
-					case BamlRecordType.StaticResourceStart:
-						NavigateTree(doc, BamlRecordType.StaticResourceStart, BamlRecordType.StaticResourceEnd, ref index);
-						keys = true;
-						break;
-					case BamlRecordType.KeyElementStart:
-						NavigateTree(doc, BamlRecordType.KeyElementStart, BamlRecordType.KeyElementEnd, ref index);
-						keys = true;
-						break;
-					default:
-						keys = false;
-						index--;
-						break;
-				}
-				index++;
-			} while (keys);
 			Record = resolve(doc[index].Position + pos);
 		}
 
 		public void WriteDefer(BamlDocument doc, int index, BinaryWriter wtr)
 		{
-			bool keys = true;
+			index = BamlDeferReader.SkipKeys(doc, index);
-			do
-			{
-				switch (doc[index].Type)
-				{
-					case BamlRecordType.DefAttributeKeyString:
-					case BamlRecordType.DefAttributeKeyType:
-					case BamlRecordType.OptimizedStaticResource:
-						keys = true;
-						break;
-					case BamlRecordType.StaticResourceStart:
-						NavigateTree(doc, BamlRecordType.StaticResourceStart, BamlRecordType.StaticResourceEnd, ref index);
-						keys = true;
-						break;
-					case BamlRecordType.KeyElementStart:
-						NavigateTree(doc, BamlRecordType.KeyElementStart, BamlRecordType.KeyElementEnd, ref index);
-						keys = true;
-						break;
-					default:
-						keys = false;
-						index--;
-						break;
-				}
-				index++;
-			} while (keys);
 			wtr.BaseStream.Seek(pos, SeekOrigin.Begin);
 			wtr.Write((uint)(Record.Position - doc[index].Position));
 		}
@@ -411,19 +436,6 @@ namespace ICSharpCode.BamlDecompiler.Baml
 			writer.Write(Shared);
 			writer.Write(SharedSet);
 		}
-
-		static void NavigateTree(BamlDocument doc, BamlRecordType start, BamlRecordType end, ref int index)
-		{
-			index++;
-			while (true) //Assume there alway is a end
-			{
-				if (doc[index].Type == start)
-					NavigateTree(doc, start, end, ref index);
-				else if (doc[index].Type == end)
-					return;
-				index++;
-			}
-		}
 	}
 
 	internal class TypeInfoRecord : SizedBamlRecord
@@ -801,61 +813,13 @@ namespace ICSharpCode.BamlDecompiler.Baml
 
 		public void ReadDefer(BamlDocument doc, int index, Func<long, BamlRecord> resolve)
 		{
-			bool keys = true;
+			index = BamlDeferReader.SkipKeys(doc, index);
-			do
-			{
-				switch (doc[index].Type)
-				{
-					case BamlRecordType.DefAttributeKeyString:
-					case BamlRecordType.DefAttributeKeyType:
-					case BamlRecordType.OptimizedStaticResource:
-						keys = true;
-						break;
-					case BamlRecordType.StaticResourceStart:
-						NavigateTree(doc, BamlRecordType.StaticResourceStart, BamlRecordType.StaticResourceEnd, ref index);
-						keys = true;
-						break;
-					case BamlRecordType.KeyElementStart:
-						NavigateTree(doc, BamlRecordType.KeyElementStart, BamlRecordType.KeyElementEnd, ref index);
-						keys = true;
-						break;
-					default:
-						keys = false;
-						index--;
-						break;
-				}
-				index++;
-			} while (keys);
 			Record = resolve(doc[index].Position + pos);
 		}
 
 		public void WriteDefer(BamlDocument doc, int index, BinaryWriter wtr)
 		{
-			bool keys = true;
+			index = BamlDeferReader.SkipKeys(doc, index);
-			do
-			{
-				switch (doc[index].Type)
-				{
-					case BamlRecordType.DefAttributeKeyString:
-					case BamlRecordType.DefAttributeKeyType:
-					case BamlRecordType.OptimizedStaticResource:
-						keys = true;
-						break;
-					case BamlRecordType.StaticResourceStart:
-						NavigateTree(doc, BamlRecordType.StaticResourceStart, BamlRecordType.StaticResourceEnd, ref index);
-						keys = true;
-						break;
-					case BamlRecordType.KeyElementStart:
-						NavigateTree(doc, BamlRecordType.KeyElementStart, BamlRecordType.KeyElementEnd, ref index);
-						keys = true;
-						break;
-					default:
-						keys = false;
-						index--;
-						break;
-				}
-				index++;
-			} while (keys);
 			wtr.BaseStream.Seek(pos, SeekOrigin.Begin);
 			wtr.Write((uint)(Record.Position - doc[index].Position));
 		}
@@ -876,19 +840,6 @@ namespace ICSharpCode.BamlDecompiler.Baml
 			writer.Write(Shared);
 			writer.Write(SharedSet);
 		}
-
-		static void NavigateTree(BamlDocument doc, BamlRecordType start, BamlRecordType end, ref int index)
-		{
-			index++;
-			while (true)
-			{
-				if (doc[index].Type == start)
-					NavigateTree(doc, start, end, ref index);
-				else if (doc[index].Type == end)
-					return;
-				index++;
-			}
-		}
 	}
 
 	internal class PropertyListStartRecord : PropertyComplexStartRecord

ILSpy.BamlDecompiler.Tests/BamlSecurityTests.cs

@@ -0,0 +1,140 @@
+// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
+//
+// 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.IO;
+using System.Text;
+
+using ICSharpCode.BamlDecompiler;
+using ICSharpCode.Decompiler.Metadata;
+
+using NUnit.Framework;
+
+namespace ILSpy.BamlDecompiler.Tests
+{
+	/// <summary>
+	/// Hardening tests for the BAML reader: a crafted or corrupt resource must surface a
+	/// single catchable exception rather than crashing the process (uncontrolled recursion
+	/// terminating with an uncatchable StackOverflowException) or allocating gigabytes.
+	/// These drive the public <see cref="XamlDecompiler.Decompile(Stream)"/> entry point,
+	/// the same one resource browsing uses.
+	/// </summary>
+	[TestFixture]
+	public class BamlSecurityTests
+	{
+		// BAML record type ids (see BamlRecordType in the decompiler).
+		const byte DefAttributeKeyString = 0x26;
+		const byte StaticResourceStart = 0x30;
+
+		static readonly string AssemblyPath = typeof(BamlSecurityTests).Assembly.Location;
+
+		static XamlDecompiler CreateDecompiler(PEFile module)
+		{
+			var resolver = new UniversalAssemblyResolver(AssemblyPath, false, module.Metadata.DetectTargetFrameworkId());
+			var typeSystem = new BamlDecompilerTypeSystem(module, resolver);
+			return new XamlDecompiler(typeSystem, new BamlDecompilerSettings());
+		}
+
+		/// <summary>
+		/// Writes the smallest header the reader accepts: the "MSBAML" signature block and
+		/// the three (reader/updater/writer) 0.0x60 version structs.
+		/// </summary>
+		static void WriteValidHeader(BinaryWriter w)
+		{
+			byte[] sig = Encoding.Unicode.GetBytes("MSBAML");
+			w.Write((uint)sig.Length);   // byte count; the reader takes len >> 1 characters
+			w.Write(sig);                // 12 bytes, already 4-byte aligned so no padding follows
+			for (int i = 0; i < 3; i++)
+			{
+				w.Write((ushort)0x0000); // Major
+				w.Write((ushort)0x0060); // Minor
+			}
+		}
+
+		static void WriteDefAttributeKeyString(BinaryWriter w)
+		{
+			w.Write(DefAttributeKeyString);
+			w.Write((byte)9);     // SizedBamlRecord size prefix (1 byte size + 8 bytes data)
+			w.Write((ushort)0);   // ValueId
+			w.Write((uint)0);     // pos
+			w.Write(false);       // Shared
+			w.Write(false);       // SharedSet
+		}
+
+		static void WriteStaticResourceStart(BinaryWriter w)
+		{
+			w.Write(StaticResourceStart);
+			w.Write((ushort)0);   // TypeId
+			w.Write((byte)0);     // Flags
+		}
+
+		[Test]
+		public void DeeplyNestedDeferBlock_DoesNotStackOverflow()
+		{
+			// A DefAttributeKeyString defer block followed by tens of thousands of nested
+			// StaticResourceStart records makes the unfixed NavigateTree recurse without a
+			// depth cap, terminating the process with an uncatchable StackOverflowException.
+			var ms = new MemoryStream();
+			using (var w = new BinaryWriter(ms, Encoding.Unicode, leaveOpen: true))
+			{
+				WriteValidHeader(w);
+				WriteDefAttributeKeyString(w);
+				for (int i = 0; i < 20000; i++)
+					WriteStaticResourceStart(w);
+			}
+			ms.Position = 0;
+
+			using var module = new PEFile(AssemblyPath);
+			Assert.Throws<InvalidDataException>(() => CreateDecompiler(module).Decompile(ms));
+		}
+
+		[Test]
+		public void UnterminatedDeferBlock_ThrowsInvalidData()
+		{
+			// A single StaticResourceStart with no matching StaticResourceEnd walks the
+			// record index off the end of the list (ArgumentOutOfRangeException before the
+			// fix); the reader must report it as malformed data instead.
+			var ms = new MemoryStream();
+			using (var w = new BinaryWriter(ms, Encoding.Unicode, leaveOpen: true))
+			{
+				WriteValidHeader(w);
+				WriteDefAttributeKeyString(w);
+				WriteStaticResourceStart(w);
+			}
+			ms.Position = 0;
+
+			using var module = new PEFile(AssemblyPath);
+			Assert.Throws<InvalidDataException>(() => CreateDecompiler(module).Decompile(ms));
+		}
+
+		[Test]
+		public void OversizedSignatureLength_ThrowsInvalidData()
+		{
+			// The signature length is attacker-controlled and read before the MSBAML check;
+			// an enormous value must be rejected before it drives a multi-gigabyte allocation.
+			var ms = new MemoryStream();
+			using (var w = new BinaryWriter(ms, Encoding.Unicode, leaveOpen: true))
+			{
+				w.Write((uint)0x7FFFFFFF);
+			}
+			ms.Position = 0;
+
+			using var module = new PEFile(AssemblyPath);
+			Assert.Throws<InvalidDataException>(() => CreateDecompiler(module).Decompile(ms));
+		}
+	}
+}

ILSpy.BamlDecompiler.Tests/ILSpy.BamlDecompiler.Tests.csproj

@@ -49,6 +49,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <Compile Include="BamlSecurityTests.cs" />
     <Compile Include="BamlTestRunner.cs" />
     <Compile Include="Cases\AttachedEvent.xaml.cs">
       <DependentUpon>AttachedEvent.xaml</DependentUpon>