From 61698b615d1333fb3184043b021ffb17f796fac6 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Wed, 25 Jul 2018 23:59:17 +0200 Subject: [PATCH] Fix #1232: BadImageFormatException: Invalid SEH header This makes the decompiler and a bunch of analyzers more robust against invalid assemblies. --- .../CSharp/CSharpDecompiler.cs | 122 ++++++++++-------- .../CSharp/RequiredNamespaceCollector.cs | 7 +- .../Disassembler/MethodBodyDisassembler.cs | 8 +- .../Analyzers/Builtin/FieldAccessAnalyzer.cs | 28 +++- .../Analyzers/Builtin/MethodUsedByAnalyzer.cs | 31 +++-- ILSpy/Analyzers/Builtin/MethodUsesAnalyzer.cs | 35 ++++- .../Builtin/MethodVirtualUsedByAnalyzer.cs | 7 +- .../Builtin/TypeInstantiatedByAnalyzer.cs | 32 +++-- ILSpy/Analyzers/IAnalyzer.cs | 6 +- ILSpy/Languages/CSharpLanguage.cs | 16 ++- 10 files changed, 187 insertions(+), 105 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index b62dd5eb9..19bb00805 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -509,64 +509,72 @@ namespace ICSharpCode.Decompiler.CSharp while (connectedMethods.Count > 0) { part = connectedMethods.Dequeue(); - var md = module.Metadata.GetMethodDefinition(part); - - if (!md.HasBody()) { - info.AddMapping(parent, part); - } else { - var blob = module.Reader.GetMethodBody(md.RelativeVirtualAddress).GetILReader(); - while (blob.RemainingBytes > 0) { - var code = blob.DecodeOpCode(); - switch (code) { - case ILOpCode.Stfld: - // async and yield fsms: - var token = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); - if (!token.IsNil && token.Kind == HandleKind.FieldDefinition) { - var fsmField = module.Metadata.GetFieldDefinition((FieldDefinitionHandle)token); - var fsmTypeDef = fsmField.GetDeclaringType(); - if (!fsmTypeDef.IsNil) { - var fsmType = module.Metadata.GetTypeDefinition(fsmTypeDef); - // Must be a nested type of the containing type. - if (fsmType.GetDeclaringType() != declaringType) - break; - if (!processedNestedTypes.Add(fsmTypeDef)) - break; - if (YieldReturnDecompiler.IsCompilerGeneratorEnumerator(fsmTypeDef, module.Metadata) - || AsyncAwaitDecompiler.IsCompilerGeneratedStateMachine(fsmTypeDef, module.Metadata)) { - foreach (var h in fsmType.GetMethods()) { - if (module.MethodSemanticsLookup.GetSemantics(h).Item2 != 0) - continue; - var otherMethod = module.Metadata.GetMethodDefinition(h); - if (!otherMethod.GetCustomAttributes().HasKnownAttribute(module.Metadata, KnownAttribute.DebuggerHidden)) { - connectedMethods.Enqueue(h); - } - } - } - } - } - break; - case ILOpCode.Ldftn: - // deal with ldftn instructions, i.e., lambdas - token = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); - if (!token.IsNil && token.Kind == HandleKind.MethodDefinition) { - if (((MethodDefinitionHandle)token).IsCompilerGenerated(module.Metadata)) - connectedMethods.Enqueue((MethodDefinitionHandle)token); - } + try { + ReadCodeMappingInfo(module, declaringType, info, parent, part, connectedMethods, processedNestedTypes); + } catch (BadImageFormatException) { + // ignore invalid IL + } + } + } + + return info; + } + + private static void ReadCodeMappingInfo(PEFile module, TypeDefinitionHandle declaringType, CodeMappingInfo info, MethodDefinitionHandle parent, MethodDefinitionHandle part, Queue connectedMethods, HashSet processedNestedTypes) + { + var md = module.Metadata.GetMethodDefinition(part); + + if (!md.HasBody()) { + info.AddMapping(parent, part); + return; + } + + var blob = module.Reader.GetMethodBody(md.RelativeVirtualAddress).GetILReader(); + while (blob.RemainingBytes > 0) { + var code = blob.DecodeOpCode(); + switch (code) { + case ILOpCode.Stfld: + // async and yield fsms: + var token = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); + if (!token.IsNil && token.Kind == HandleKind.FieldDefinition) { + var fsmField = module.Metadata.GetFieldDefinition((FieldDefinitionHandle)token); + var fsmTypeDef = fsmField.GetDeclaringType(); + if (!fsmTypeDef.IsNil) { + var fsmType = module.Metadata.GetTypeDefinition(fsmTypeDef); + // Must be a nested type of the containing type. + if (fsmType.GetDeclaringType() != declaringType) break; - default: - blob.SkipOperand(code); + if (!processedNestedTypes.Add(fsmTypeDef)) break; + if (YieldReturnDecompiler.IsCompilerGeneratorEnumerator(fsmTypeDef, module.Metadata) + || AsyncAwaitDecompiler.IsCompilerGeneratedStateMachine(fsmTypeDef, module.Metadata)) { + foreach (var h in fsmType.GetMethods()) { + if (module.MethodSemanticsLookup.GetSemantics(h).Item2 != 0) + continue; + var otherMethod = module.Metadata.GetMethodDefinition(h); + if (!otherMethod.GetCustomAttributes().HasKnownAttribute(module.Metadata, KnownAttribute.DebuggerHidden)) { + connectedMethods.Enqueue(h); + } + } + } } } - - info.AddMapping(parent, part); - } + break; + case ILOpCode.Ldftn: + // deal with ldftn instructions, i.e., lambdas + token = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); + if (!token.IsNil && token.Kind == HandleKind.MethodDefinition) { + if (((MethodDefinitionHandle)token).IsCompilerGenerated(module.Metadata)) + connectedMethods.Enqueue((MethodDefinitionHandle)token); + } + break; + default: + blob.SkipOperand(code); + break; } - - } - return info; + info.AddMapping(parent, part); } /// @@ -1047,7 +1055,16 @@ namespace ICSharpCode.Decompiler.CSharp DebugInfo = DebugInfoProvider }; var methodDef = metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken); - var methodBody = module.PEFile.Reader.GetMethodBody(methodDef.RelativeVirtualAddress); + var body = BlockStatement.Null; + MethodBodyBlock methodBody; + try { + methodBody = module.PEFile.Reader.GetMethodBody(methodDef.RelativeVirtualAddress); + } catch (BadImageFormatException ex) { + body = new BlockStatement(); + body.AddChild(new Comment("Invalid MethodBodyBlock: " + ex.Message), Roles.Comment); + entityDecl.AddChild(body, Roles.Body); + return; + } var function = ilReader.ReadIL((MethodDefinitionHandle)method.MetadataToken, methodBody, cancellationToken: CancellationToken); function.CheckInvariant(ILPhase.Normal); @@ -1083,7 +1100,6 @@ namespace ICSharpCode.Decompiler.CSharp break; } - var body = BlockStatement.Null; // Generate C# AST only if bodies should be displayed. if (localSettings.DecompileMemberBodies) { AddDefinesForConditionalAttributes(function, decompileRun); diff --git a/ICSharpCode.Decompiler/CSharp/RequiredNamespaceCollector.cs b/ICSharpCode.Decompiler/CSharp/RequiredNamespaceCollector.cs index f31d3727a..02bc5584b 100644 --- a/ICSharpCode.Decompiler/CSharp/RequiredNamespaceCollector.cs +++ b/ICSharpCode.Decompiler/CSharp/RequiredNamespaceCollector.cs @@ -93,7 +93,12 @@ namespace ICSharpCode.Decompiler.CSharp var parts = mappingInfo.GetMethodParts((MethodDefinitionHandle)method.MetadataToken).ToList(); foreach (var part in parts) { var methodDef = module.metadata.GetMethodDefinition(part); - var body = reader.GetMethodBody(methodDef.RelativeVirtualAddress); + MethodBodyBlock body; + try { + body = reader.GetMethodBody(methodDef.RelativeVirtualAddress); + } catch (BadImageFormatException) { + continue; + } CollectNamespacesFromMethodBody(body, module, namespaces); } } diff --git a/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs b/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs index 8ac325134..49b0661ae 100644 --- a/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs +++ b/ICSharpCode.Decompiler/Disassembler/MethodBodyDisassembler.cs @@ -88,7 +88,13 @@ namespace ICSharpCode.Decompiler.Disassembler output.WriteLine(); return; } - var body = module.Reader.GetMethodBody(methodDefinition.RelativeVirtualAddress); + MethodBodyBlock body; + try { + body = module.Reader.GetMethodBody(methodDefinition.RelativeVirtualAddress); + } catch (BadImageFormatException ex) { + output.WriteLine("// {0}", ex.Message); + return; + } var blob = body.GetILReader(); output.WriteLine("// Code size {0} (0x{0:x})", blob.Length); output.WriteLine(".maxstack {0}", body.MaxStack); diff --git a/ILSpy/Analyzers/Builtin/FieldAccessAnalyzer.cs b/ILSpy/Analyzers/Builtin/FieldAccessAnalyzer.cs index e3b414423..906b6acf1 100644 --- a/ILSpy/Analyzers/Builtin/FieldAccessAnalyzer.cs +++ b/ILSpy/Analyzers/Builtin/FieldAccessAnalyzer.cs @@ -119,7 +119,13 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin foreach (var part in mappingInfo.GetMethodParts((MethodDefinitionHandle)method.MetadataToken)) { var md = module.Metadata.GetMethodDefinition(part); if (!md.HasBody()) continue; - if (ScanMethodBody(analyzedField, method, module.Reader.GetMethodBody(md.RelativeVirtualAddress))) + MethodBodyBlock body; + try { + body = module.Reader.GetMethodBody(md.RelativeVirtualAddress); + } catch (BadImageFormatException) { + return false; + } + if (ScanMethodBody(analyzedField, method, body)) return true; } return false; @@ -135,15 +141,25 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin var genericContext = new Decompiler.TypeSystem.GenericContext(); // type parameters don't matter for this analyzer while (blob.RemainingBytes > 0) { - var opCode = blob.DecodeOpCode(); - if (!CanBeReference(opCode)) { - blob.SkipOperand(opCode); - continue; + ILOpCode opCode; + try { + opCode = blob.DecodeOpCode(); + if (!CanBeReference(opCode)) { + blob.SkipOperand(opCode); + continue; + } + } catch (BadImageFormatException) { + return false; } EntityHandle fieldHandle = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); if (!fieldHandle.Kind.IsMemberKind()) continue; - var field = mainModule.ResolveEntity(fieldHandle, genericContext) as IField; + IField field; + try { + field = mainModule.ResolveEntity(fieldHandle, genericContext) as IField; + } catch (BadImageFormatException) { + continue; + } if (field == null) continue; diff --git a/ILSpy/Analyzers/Builtin/MethodUsedByAnalyzer.cs b/ILSpy/Analyzers/Builtin/MethodUsedByAnalyzer.cs index c89424c05..4b2843065 100644 --- a/ILSpy/Analyzers/Builtin/MethodUsedByAnalyzer.cs +++ b/ILSpy/Analyzers/Builtin/MethodUsedByAnalyzer.cs @@ -16,6 +16,7 @@ // 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.ComponentModel.Composition; using System.Diagnostics; @@ -80,12 +81,7 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin bool IsUsedInMethod(IMethod analyzedEntity, IMethod method, CodeMappingInfo mappingInfo, AnalyzerContext context) { - if (method.MetadataToken.IsNil) - return false; - var module = method.ParentModule.PEFile; - var md = module.Metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken); - if (!md.HasBody()) return false; - return ScanMethodBody(analyzedEntity, method, module.Reader.GetMethodBody(md.RelativeVirtualAddress)); + return ScanMethodBody(analyzedEntity, method, context.GetMethodBody(method)); } static bool ScanMethodBody(IMethod analyzedMethod, IMethod method, MethodBodyBlock methodBody) @@ -100,16 +96,27 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin var genericContext = new Decompiler.TypeSystem.GenericContext(); // type parameters don't matter for this analyzer while (blob.RemainingBytes > 0) { - var opCode = blob.DecodeOpCode(); - if (opCode != ILOpCode.Call && opCode != ILOpCode.Callvirt) { - ILParser.SkipOperand(ref blob, opCode); - continue; + ILOpCode opCode; + try { + opCode = blob.DecodeOpCode(); + if (opCode != ILOpCode.Call && opCode != ILOpCode.Callvirt) { + ILParser.SkipOperand(ref blob, opCode); + continue; + } + } catch (BadImageFormatException) { + return false; // unexpected end of blob } var member = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); if (member.IsNil || !member.Kind.IsMemberKind()) continue; - var m = (mainModule.ResolveEntity(member, genericContext) as IMember)?.MemberDefinition; - if (m == null) continue; + IMember m; + try { + m = (mainModule.ResolveEntity(member, genericContext) as IMember)?.MemberDefinition; + } catch (BadImageFormatException) { + continue; + } + if (m == null) + continue; if (opCode == ILOpCode.Call) { if (IsSameMember(analyzedMethod, m)) { diff --git a/ILSpy/Analyzers/Builtin/MethodUsesAnalyzer.cs b/ILSpy/Analyzers/Builtin/MethodUsesAnalyzer.cs index 8c6436890..80ddd0b5b 100644 --- a/ILSpy/Analyzers/Builtin/MethodUsesAnalyzer.cs +++ b/ILSpy/Analyzers/Builtin/MethodUsesAnalyzer.cs @@ -54,12 +54,22 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin var md = module.PEFile.Metadata.GetMethodDefinition(handle); if (!md.HasBody()) yield break; - var blob = module.PEFile.Reader.GetMethodBody(md.RelativeVirtualAddress).GetILReader(); + BlobReader blob; + try { + blob = module.PEFile.Reader.GetMethodBody(md.RelativeVirtualAddress).GetILReader(); + } catch (BadImageFormatException) { + yield break; + } var visitor = new TypeDefinitionCollector(); var genericContext = new Decompiler.TypeSystem.GenericContext(); // type parameters don't matter for this analyzer while (blob.RemainingBytes > 0) { - var opCode = blob.DecodeOpCode(); + ILOpCode opCode; + try { + opCode = blob.DecodeOpCode(); + } catch (BadImageFormatException) { + yield break; + } switch (opCode.GetOperandType()) { case OperandType.Field: case OperandType.Method: @@ -74,20 +84,35 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin case HandleKind.TypeDefinition: case HandleKind.TypeReference: case HandleKind.TypeSpecification: - module.ResolveType(member, genericContext).AcceptVisitor(visitor); + IType ty; + try { + ty = module.ResolveType(member, genericContext); + } catch (BadImageFormatException) { + ty = null; + } + ty?.AcceptVisitor(visitor); break; case HandleKind.MethodDefinition: case HandleKind.MethodSpecification: case HandleKind.MemberReference: case HandleKind.FieldDefinition: - var m = module.ResolveEntity(member, genericContext); + IEntity m; + try { + m = module.ResolveEntity(member, genericContext); + } catch (BadImageFormatException) { + m = null; + } if (m != null) yield return m; break; } break; default: - ILParser.SkipOperand(ref blob, opCode); + try { + ILParser.SkipOperand(ref blob, opCode); + } catch (BadImageFormatException) { + yield break; + } break; } } diff --git a/ILSpy/Analyzers/Builtin/MethodVirtualUsedByAnalyzer.cs b/ILSpy/Analyzers/Builtin/MethodVirtualUsedByAnalyzer.cs index 8bdc3aee5..b03d804e5 100644 --- a/ILSpy/Analyzers/Builtin/MethodVirtualUsedByAnalyzer.cs +++ b/ILSpy/Analyzers/Builtin/MethodVirtualUsedByAnalyzer.cs @@ -81,12 +81,7 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin bool IsUsedInMethod(IMethod analyzedEntity, IMethod method, CodeMappingInfo mappingInfo, AnalyzerContext context) { - if (method.MetadataToken.IsNil) - return false; - var module = method.ParentModule.PEFile; - var md = module.Metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken); - if (!md.HasBody()) return false; - return ScanMethodBody(analyzedEntity, method, module.Reader.GetMethodBody(md.RelativeVirtualAddress)); + return ScanMethodBody(analyzedEntity, method, context.GetMethodBody(method)); } static bool ScanMethodBody(IMethod analyzedMethod, IMethod method, MethodBodyBlock methodBody) diff --git a/ILSpy/Analyzers/Builtin/TypeInstantiatedByAnalyzer.cs b/ILSpy/Analyzers/Builtin/TypeInstantiatedByAnalyzer.cs index cfb9252d6..a8a7e9794 100644 --- a/ILSpy/Analyzers/Builtin/TypeInstantiatedByAnalyzer.cs +++ b/ILSpy/Analyzers/Builtin/TypeInstantiatedByAnalyzer.cs @@ -81,31 +81,37 @@ namespace ICSharpCode.ILSpy.Analyzers.Builtin bool IsUsedInMethod(ITypeDefinition analyzedEntity, IMethod method, CodeMappingInfo mappingInfo, AnalyzerContext context) { - if (method.MetadataToken.IsNil) - return false; - var module = method.ParentModule.PEFile; - var md = module.Metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken); - if (!md.HasBody()) return false; - return ScanMethodBody(analyzedEntity, method, module.Reader.GetMethodBody(md.RelativeVirtualAddress)); + return ScanMethodBody(analyzedEntity, method, context.GetMethodBody(method)); } bool ScanMethodBody(ITypeDefinition analyzedEntity, IMethod method, MethodBodyBlock methodBody) { - bool found = false; + if (methodBody == null) + return false; var blob = methodBody.GetILReader(); var module = (MetadataModule)method.ParentModule; var genericContext = new Decompiler.TypeSystem.GenericContext(); // type parameters don't matter for this analyzer - while (!found && blob.RemainingBytes > 0) { - var opCode = blob.DecodeOpCode(); - if (!CanBeReference(opCode)) { - blob.SkipOperand(opCode); - continue; + while (blob.RemainingBytes > 0) { + ILOpCode opCode; + try { + opCode = blob.DecodeOpCode(); + if (!CanBeReference(opCode)) { + blob.SkipOperand(opCode); + continue; + } + } catch (BadImageFormatException) { + return false; } EntityHandle methodHandle = MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()); if (!methodHandle.Kind.IsMemberKind()) continue; - var ctor = module.ResolveMethod(methodHandle, genericContext); + IMethod ctor; + try { + ctor = module.ResolveMethod(methodHandle, genericContext); + } catch (BadImageFormatException) { + continue; + } if (ctor == null || !ctor.IsConstructor) continue; diff --git a/ILSpy/Analyzers/IAnalyzer.cs b/ILSpy/Analyzers/IAnalyzer.cs index 24f15174b..2e9a6012a 100644 --- a/ILSpy/Analyzers/IAnalyzer.cs +++ b/ILSpy/Analyzers/IAnalyzer.cs @@ -68,7 +68,11 @@ namespace ICSharpCode.ILSpy.Analyzers return null; var module = method.ParentModule.PEFile; var md = module.Metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken); - return module.Reader.GetMethodBody(md.RelativeVirtualAddress); + try { + return module.Reader.GetMethodBody(md.RelativeVirtualAddress); + } catch (BadImageFormatException) { + return null; + } } public AnalyzerScope GetScopeOf(IEntity entity) diff --git a/ILSpy/Languages/CSharpLanguage.cs b/ILSpy/Languages/CSharpLanguage.cs index ebcdf951e..9096ea5d4 100644 --- a/ILSpy/Languages/CSharpLanguage.cs +++ b/ILSpy/Languages/CSharpLanguage.cs @@ -450,7 +450,7 @@ namespace ICSharpCode.ILSpy string simple = field.Name + " : " + TypeToString(field.Type, includeNamespace); if (!includeDeclaringTypeName) return simple; - return TypeToStringInternal(field.DeclaringTypeDefinition, includeNamespaceOfDeclaringTypeName) + "." + simple; + return TypeToStringInternal(field.DeclaringType, includeNamespaceOfDeclaringTypeName) + "." + simple; } public override string PropertyToString(IProperty property, bool includeDeclaringTypeName, bool includeNamespace, bool includeNamespaceOfDeclaringTypeName) @@ -458,6 +458,10 @@ namespace ICSharpCode.ILSpy if (property == null) throw new ArgumentNullException(nameof(property)); var buffer = new System.Text.StringBuilder(); + if (includeDeclaringTypeName) { + buffer.Append(TypeToString(property.DeclaringType, includeNamespaceOfDeclaringTypeName)); + buffer.Append('.'); + } if (property.IsIndexer) { if (property.IsExplicitInterfaceImplementation) { string name = property.Name; @@ -484,9 +488,7 @@ namespace ICSharpCode.ILSpy } buffer.Append(" : "); buffer.Append(TypeToStringInternal(property.ReturnType, includeNamespace)); - if (!includeDeclaringTypeName) - return buffer.ToString(); - return TypeToString(property.DeclaringTypeDefinition, includeNamespaceOfDeclaringTypeName) + "." + buffer.ToString(); + return buffer.ToString(); } public override string MethodToString(IMethod method, bool includeDeclaringTypeName, bool includeNamespace, bool includeNamespaceOfDeclaringTypeName) @@ -495,12 +497,12 @@ namespace ICSharpCode.ILSpy throw new ArgumentNullException(nameof(method)); string name; if (includeDeclaringTypeName) { - name = TypeToString(method.DeclaringTypeDefinition, includeNamespace: includeNamespaceOfDeclaringTypeName) + "."; + name = TypeToString(method.DeclaringType, includeNamespace: includeNamespaceOfDeclaringTypeName) + "."; } else { name = ""; } if (method.IsConstructor) { - name += TypeToString(method.DeclaringTypeDefinition, false); + name += TypeToString(method.DeclaringType, false); } else { name += method.Name; } @@ -542,7 +544,7 @@ namespace ICSharpCode.ILSpy throw new ArgumentNullException(nameof(@event)); var buffer = new System.Text.StringBuilder(); if (includeDeclaringTypeName) { - buffer.Append(TypeToString(@event.DeclaringTypeDefinition, includeNamespaceOfDeclaringTypeName) + "."); + buffer.Append(TypeToString(@event.DeclaringType, includeNamespaceOfDeclaringTypeName) + "."); } buffer.Append(@event.Name); buffer.Append(" : ");