From fa194704353a768f023147d8b4a075017329aab5 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 15 Jun 2024 15:03:21 +0200 Subject: [PATCH 1/2] Fix #3218: Avoid exceptions when IL is invalid due to unexpected end-of-method-body. --- .../Disassembler/ILParser.cs | 98 +++++++++++++++---- .../Disassembler/ILStructure.cs | 2 + .../Disassembler/MethodBodyDisassembler.cs | 3 +- ICSharpCode.Decompiler/IL/ILReader.cs | 10 +- 4 files changed, 92 insertions(+), 21 deletions(-) diff --git a/ICSharpCode.Decompiler/Disassembler/ILParser.cs b/ICSharpCode.Decompiler/Disassembler/ILParser.cs index 3340c82a7..149568777 100644 --- a/ICSharpCode.Decompiler/Disassembler/ILParser.cs +++ b/ICSharpCode.Decompiler/Disassembler/ILParser.cs @@ -30,18 +30,24 @@ namespace ICSharpCode.Decompiler.Disassembler public static ILOpCode DecodeOpCode(this ref BlobReader blob) { byte opCodeByte = blob.ReadByte(); - return (ILOpCode)(opCodeByte == 0xFE ? 0xFE00 + blob.ReadByte() : opCodeByte); + if (opCodeByte == 0xFE && blob.RemainingBytes >= 1) + { + return (ILOpCode)(0xFE00 + blob.ReadByte()); + } + else + { + return (ILOpCode)opCodeByte; + } } - public static void SkipOperand(this ref BlobReader blob, ILOpCode opCode) + internal static int OperandSize(this OperandType opType) { - switch (opCode.GetOperandType()) + switch (opType) { // 64-bit case OperandType.I8: case OperandType.R: - blob.Offset += 8; - break; + return 8; // 32-bit case OperandType.BrTarget: case OperandType.Field: @@ -52,37 +58,91 @@ namespace ICSharpCode.Decompiler.Disassembler case OperandType.Tok: case OperandType.Type: case OperandType.ShortR: - blob.Offset += 4; - break; + return 4; // (n + 1) * 32-bit case OperandType.Switch: - uint n = blob.ReadUInt32(); - blob.Offset += (int)(n * 4); - break; - // 16-bit - case OperandType.Variable: - blob.Offset += 2; - break; + return 4; // minimum 4, usually more + case OperandType.Variable: // 16-bit + return 2; // 8-bit case OperandType.ShortVariable: case OperandType.ShortBrTarget: case OperandType.ShortI: - blob.Offset++; - break; + return 1; + default: + return 0; + } + } + + public static void SkipOperand(this ref BlobReader blob, ILOpCode opCode) + { + var opType = opCode.GetOperandType(); + int operandSize; + if (opType == OperandType.Switch) + { + uint n = blob.RemainingBytes >= 4 ? blob.ReadUInt32() : uint.MaxValue; + if (n < int.MaxValue / 4) + { + operandSize = (int)(n * 4); + } + else + { + operandSize = int.MaxValue; + } + } + else + { + operandSize = opType.OperandSize(); + } + if (operandSize <= blob.RemainingBytes) + { + blob.Offset += operandSize; + } + else + { + // ignore missing/partial operand at end of body + blob.Offset = blob.Length; } } public static int DecodeBranchTarget(this ref BlobReader blob, ILOpCode opCode) { - return (opCode.GetBranchOperandSize() == 4 ? blob.ReadInt32() : blob.ReadSByte()) + blob.Offset; + int opSize = opCode.GetBranchOperandSize(); + if (opSize <= blob.RemainingBytes) + { + int relOffset = opSize == 4 ? blob.ReadInt32() : blob.ReadSByte(); + return unchecked(relOffset + blob.Offset); + } + else + { + return int.MinValue; + } } public static int[] DecodeSwitchTargets(this ref BlobReader blob) { - int[] targets = new int[blob.ReadUInt32()]; + if (blob.RemainingBytes < 4) + { + blob.Offset += blob.RemainingBytes; + return new int[0]; + } + uint numTargets = blob.ReadUInt32(); + bool numTargetOverflow = false; + if (numTargets > blob.RemainingBytes / 4) + { + numTargets = (uint)(blob.RemainingBytes / 4); + numTargetOverflow = true; + } + int[] targets = new int[numTargets]; int offset = blob.Offset + 4 * targets.Length; for (int i = 0; i < targets.Length; i++) - targets[i] = blob.ReadInt32() + offset; + { + targets[i] = unchecked(blob.ReadInt32() + offset); + } + if (numTargetOverflow) + { + blob.Offset += blob.RemainingBytes; + } return targets; } diff --git a/ICSharpCode.Decompiler/Disassembler/ILStructure.cs b/ICSharpCode.Decompiler/Disassembler/ILStructure.cs index f73b31763..6993ed7ff 100644 --- a/ICSharpCode.Decompiler/Disassembler/ILStructure.cs +++ b/ICSharpCode.Decompiler/Disassembler/ILStructure.cs @@ -171,6 +171,8 @@ namespace ICSharpCode.Decompiler.Disassembler // special case: don't consider the loop-like structure of "continue;" statements to be nested loops if (this.Type == ILStructureType.Loop && newStructure.Type == ILStructureType.Loop && newStructure.StartOffset == this.StartOffset) return false; + if (newStructure.StartOffset < 0) + return false; // use <= for end-offset comparisons because both end and EndOffset are exclusive Debug.Assert(StartOffset <= newStructure.StartOffset && newStructure.EndOffset <= EndOffset); diff --git a/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs b/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs index c61179211..fbd386d4f 100644 --- a/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs +++ b/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs @@ -350,7 +350,8 @@ namespace ICSharpCode.Decompiler.Disassembler } } ILOpCode opCode = ILParser.DecodeOpCode(ref blob); - if (opCode.IsDefined()) + OperandType opType = opCode.GetOperandType(); + if (opCode.IsDefined() && opType.OperandSize() <= blob.RemainingBytes) { WriteRVA(blob, offset + methodRva, opCode); output.WriteLocalReference(DisassemblerHelpers.OffsetToString(offset), offset, isDefinition: true); diff --git a/ICSharpCode.Decompiler/IL/ILReader.cs b/ICSharpCode.Decompiler/IL/ILReader.cs index 21f8e5290..ca1ed07c1 100644 --- a/ICSharpCode.Decompiler/IL/ILReader.cs +++ b/ICSharpCode.Decompiler/IL/ILReader.cs @@ -1780,7 +1780,15 @@ namespace ICSharpCode.Decompiler.IL DecodedInstruction DecodeCallIndirect() { - var signatureHandle = (StandaloneSignatureHandle)ReadAndDecodeMetadataToken(); + StandaloneSignatureHandle signatureHandle; + try + { + signatureHandle = (StandaloneSignatureHandle)ReadAndDecodeMetadataToken(); + } + catch (InvalidCastException ex) + { + throw new BadImageFormatException("Invalid calli metadata token", ex); + } var (header, fpt) = module.DecodeMethodSignature(signatureHandle, genericContext); var functionPointer = Pop(StackType.I); int firstArgument = header.IsInstance ? 1 : 0; From 476e80b190180a8a4d6800304999d2aa3dc022ec Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 16 Jun 2024 17:37:47 +0200 Subject: [PATCH 2/2] Use uncompressedStream for PEFile in XamarinCompressedFileLoader --- ICSharpCode.ILSpyX/FileLoaders/XamarinCompressedFileLoader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ICSharpCode.ILSpyX/FileLoaders/XamarinCompressedFileLoader.cs b/ICSharpCode.ILSpyX/FileLoaders/XamarinCompressedFileLoader.cs index 349822f41..a3b66ee7d 100644 --- a/ICSharpCode.ILSpyX/FileLoaders/XamarinCompressedFileLoader.cs +++ b/ICSharpCode.ILSpyX/FileLoaders/XamarinCompressedFileLoader.cs @@ -59,7 +59,7 @@ namespace ICSharpCode.ILSpyX.FileLoaders : MetadataReaderOptions.None; return new LoadResult { - MetadataFile = new PEFile(fileName, stream, PEStreamOptions.PrefetchEntireImage, metadataOptions: options) + MetadataFile = new PEFile(fileName, uncompressedStream, PEStreamOptions.PrefetchEntireImage, metadataOptions: options) }; } }