icsharpcode / ILSpy

.NET Decompiler with support for PDB generation, ReadyToRun, Metadata (&more) - cross-platform!
21.19k stars 3.32k forks source link

SRM port: System.Reflection.Metadata.MetadataReader chokes on obfuscated assembly more than Cecil #1069

Closed siegfriedpammer closed 6 years ago

siegfriedpammer commented 6 years ago

While doing some testing of the new disassembler on the srm branch, I came across this exception:

System.BadImageFormatException: Illegal tables in compressed metadata stream.
   at System.Reflection.Metadata.MetadataReader.ReadMetadataTableHeader(BlobReader& reader, HeapSizes& heapSizes, Int32[]& metadataTableRowCounts, TableMask& sortedTables)
   at System.Reflection.Metadata.MetadataReader..ctor(Byte* metadata, Int32 length, MetadataReaderOptions options, MetadataStringDecoder utf8Decoder)
   at System.Reflection.Metadata.PEReaderExtensions.GetMetadataReader(PEReader peReader, MetadataReaderOptions options, MetadataStringDecoder utf8Decoder)
   at ICSharpCode.Decompiler.Metadata.PEFile.GetMetadataReader() in C:\Users\Siegfried\Projects\ILSpy\ICSharpCode.Decompiler\Metadata\Dom.cs:line 100
   at ICSharpCode.ILSpy.ILLanguage.DecompileAssembly(LoadedAssembly assembly, ITextOutput output, DecompilationOptions options) in C:\Users\Siegfried\Projects\ILSpy\ILSpy\Languages\ILLanguage.cs:line 133
   at ICSharpCode.ILSpy.TreeNodes.AssemblyTreeNode.Decompile(Language language, ITextOutput output, DecompilationOptions options) in C:\Users\Siegfried\Projects\ILSpy\ILSpy\TreeNodes\AssemblyTreeNode.cs:line 285
   at ICSharpCode.ILSpy.TextView.DecompilerTextView.DecompileNodes(DecompilationContext context, ITextOutput textOutput) in C:\Users\Siegfried\Projects\ILSpy\ILSpy\TextView\DecompilerTextView.cs:line 529
   at ICSharpCode.ILSpy.TextView.DecompilerTextView.<>c__DisplayClass33_0.<DecompileAsync>b__0() in C:\Users\Siegfried\Projects\ILSpy\ILSpy\TextView\DecompilerTextView.cs:line 508

MetadataReader is not very resilient. Feeding it the obfuscated Lidgren.Network.dll Lidgren.Network.dll.zip makes it fail completely.

@sharwell @nguerrera @tmat Do you have any plans on improving SRM in this regard?

tmat commented 6 years ago

By design SRM does not accept assemblies that do not comply with ECMA335 spec.

Reading obfuscated assemblies doesn't seem like an important scenario for ILSpy to me. Customers using obfuscators certainly don't want their code to be disassembled.

siegfriedpammer commented 6 years ago

For ILSpy this is not much of a problem, because if ILSpy fails you can still use a deobfuscator like de4dot, but if SRM fails, how do you process these in Visual Studio, given that Roslyn uses SRM as well?

Note that this exception occurs simply when trying to open the assembly, not reading any method bodies, so retrieving public types and members would also fail -> no intellisense.

sharwell commented 6 years ago

The output of an obfuscator is still required to adhere to the specifications for the underlying VM. If it fails to do so, downstream tools are expected to fail randomly.

Before determining how how SRM should handle this, we need to determine if this is a bug in SRM (obfuscated code adheres to the specification), or if this is an intended behavior of the obfuscator (does not adhere to the specification).

sharwell commented 6 years ago

@siegfriedpammer your example produces a CS0009 compilation error if you try and compile against the assembly. IntelliSense fails as well. For all practical purposes, this is an unusable assembly from a developer's perspective.

siegfriedpammer commented 6 years ago

OK, then it is completely worthless and I think we can live with an exception in this case. Thank you for looking into this!