Enable analysis for managed binary #335
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,11 +40,6 @@ public PEBinary( | |
|
|
||
| private Pdb LoadPdb() | ||
| { | ||
| Pdb pdb = null; | ||
| try | ||
| { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,9 @@ | |
| using System.Reflection.PortableExecutable; | ||
| using System.Security.Cryptography; | ||
|
|
||
| using Microsoft.DiaSymReader; | ||
| using Microsoft.DiaSymReader.Tools; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.BinaryParsers.PortableExecutable | ||
| { | ||
| public class PE : IDisposable | ||
|
|
@@ -262,7 +265,6 @@ public byte[] ImageBytes | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Calculate SHA1 over the file contents | ||
| /// </summary> | ||
|
|
@@ -365,7 +367,6 @@ public long Length | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Windows OS file version information | ||
| /// </summary> | ||
|
|
@@ -381,7 +382,6 @@ public FileVersionInfo FileVersion | |
| } | ||
| } | ||
|
|
||
| public Packer Packer | ||
| { | ||
| get | ||
|
|
@@ -789,5 +789,66 @@ public bool IsWixBinary | |
| return this.isWixBinary.Value; | ||
| } | ||
| } | ||
|
|
||
| public bool IsChecksumAlgorithmSecureForPortablePdb() | ||
|
There was a problem hiding this comment.
rather than creating these helpers, you should add a helper that simply returns the checksum algorithm for managed PDBs. ManagedPdbSourceFileChecksumAlgorithm { get; } There was a problem hiding this comment. This property s/be on the PDB class, not in the PE class. In reply to: 563213952 [](ancestors = 563213952) There was a problem hiding this comment. oh...remembered why i didn't move the below logic to pdb.cs: to do th reading I need a few things: peReader and the metadatareader. In reply to: 563214137 [](ancestors = 563214137,563213952) There was a problem hiding this comment. Changed a little bit, let me know what do u think. In reply to: 563297494 [](ancestors = 563297494,563214137,563213952) |
||
| { | ||
| const string sha256 = "8829d00f-11b8-4213-878b-770e8597ac16"; | ||
| var sha256guid = new Guid(sha256); | ||
|
|
||
| if (this.peReader.TryOpenAssociatedPortablePdb( | ||
|
There was a problem hiding this comment.
eventually pdb location logic needs to understand binskim's '--local-symbol-directories' command-line arguments. let's open an issue and take this in a future change. #Closed |
||
| this.FileName, | ||
| filePath => File.Exists(filePath) ? File.OpenRead(filePath) : null, | ||
| out MetadataReaderProvider pdbProvider, | ||
| out _)) | ||
| { | ||
| MetadataReader metadataReader = pdbProvider.GetMetadataReader(); | ||
| foreach (DocumentHandle document in metadataReader.Documents) | ||
| { | ||
| Document doc = metadataReader.GetDocument(document); | ||
|
|
||
| Guid hashGuid = metadataReader.GetGuid(doc.HashAlgorithm); | ||
|
|
||
| return hashGuid == sha256guid; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
|
There was a problem hiding this comment.
Always prefer early exit if possible. So, change the code to say if (!TryOpen) then return false immediately. #Closed |
||
| } | ||
|
|
||
| public bool IsChecksumAlgorithmSecureForFullPdb() | ||
| { | ||
| const string sha256 = "8829d00f-11b8-4213-878b-770e8597ac16"; | ||
| var sha256guid = new Guid(sha256); | ||
|
There was a problem hiding this comment. declare a private static readonly Guid that's initialized to this. #Closed |
||
| string fileName = Path.GetFileName(this.FileName); | ||
| string extension = Path.GetExtension(fileName); | ||
| string pdbPath = this.FileName.Replace(fileName, fileName.Replace(extension, ".pdb")); | ||
|
|
||
| if (!File.Exists(pdbPath)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| using var pdbStream = new FileStream(pdbPath, FileMode.Open, FileAccess.Read); | ||
|
|
||
| var metadataProvider = new SymMetadataProvider(this.metadataReader); | ||
| object importer = SymUnmanagedReaderFactory.CreateSymReaderMetadataImport(metadataProvider); | ||
| ISymUnmanagedReader3 reader = SymUnmanagedReaderFactory.CreateReaderWithMetadataImport<ISymUnmanagedReader3>(pdbStream, importer, SymUnmanagedReaderCreationOptions.UseComRegistry); | ||
|
|
||
| try | ||
| { | ||
| Guid algorithm = Guid.Empty; | ||
| foreach (ISymUnmanagedDocument document in reader.GetDocuments()) | ||
| { | ||
| document.GetChecksumAlgorithmId(ref algorithm); | ||
| return algorithm == sha256guid; | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| _ = ((ISymUnmanagedDispose)reader).Destroy(); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Reflection; | ||
| using System.Reflection.Metadata; | ||
| using System.Reflection.Metadata.Ecma335; | ||
|
|
||
| namespace Microsoft.DiaSymReader.Tools | ||
| { | ||
| internal sealed class SymMetadataProvider : ISymWriterMetadataProvider, ISymReaderMetadataProvider | ||
| { | ||
| private readonly MetadataReader _reader; | ||
|
|
||
| internal SymMetadataProvider(MetadataReader reader) | ||
| { | ||
| _reader = reader; | ||
| } | ||
|
|
||
| public unsafe bool TryGetStandaloneSignature(int standaloneSignatureToken, out byte* signature, out int length) | ||
| { | ||
| var sigHandle = (StandaloneSignatureHandle)MetadataTokens.Handle(standaloneSignatureToken); | ||
| if (sigHandle.IsNil) | ||
| { | ||
| signature = null; | ||
| length = 0; | ||
| return false; | ||
| } | ||
|
|
||
| StandaloneSignature sig = _reader.GetStandaloneSignature(sigHandle); | ||
| BlobReader blobReader = _reader.GetBlobReader(sig.Signature); | ||
|
|
||
| signature = blobReader.StartPointer; | ||
| length = blobReader.Length; | ||
| return true; | ||
| } | ||
|
|
||
| public bool TryGetTypeDefinitionInfo(int typeDefinitionToken, [NotNullWhen(true)] out string namespaceName, [NotNullWhen(true)] out string typeName, out TypeAttributes attributes) | ||
| { | ||
| var handle = (TypeDefinitionHandle)MetadataTokens.Handle(typeDefinitionToken); | ||
| if (handle.IsNil) | ||
| { | ||
| namespaceName = null; | ||
| typeName = null; | ||
| attributes = 0; | ||
| return false; | ||
| } | ||
|
|
||
| TypeDefinition typeDefinition = _reader.GetTypeDefinition(handle); | ||
| namespaceName = _reader.GetString(typeDefinition.Namespace); | ||
| typeName = _reader.GetString(typeDefinition.Name); | ||
| attributes = typeDefinition.Attributes; | ||
| return true; | ||
| } | ||
|
|
||
| public bool TryGetTypeReferenceInfo(int typeReferenceToken, [NotNullWhen(true)] out string namespaceName, [NotNullWhen(true)] out string typeName) | ||
| { | ||
| var handle = (TypeReferenceHandle)MetadataTokens.Handle(typeReferenceToken); | ||
| if (handle.IsNil) | ||
| { | ||
| namespaceName = null; | ||
| typeName = null; | ||
| return false; | ||
| } | ||
|
|
||
| TypeReference typeReference = _reader.GetTypeReference(handle); | ||
| namespaceName = _reader.GetString(typeReference.Namespace); | ||
| typeName = _reader.GetString(typeReference.Name); | ||
| return true; | ||
| } | ||
|
|
||
| public bool TryGetEnclosingType(int nestedTypeToken, out int enclosingTypeToken) | ||
| { | ||
| TypeDefinition nestedTypeDef = _reader.GetTypeDefinition(MetadataTokens.TypeDefinitionHandle(nestedTypeToken)); | ||
| TypeDefinitionHandle declaringTypeHandle = nestedTypeDef.GetDeclaringType(); | ||
|
|
||
| if (declaringTypeHandle.IsNil) | ||
| { | ||
| enclosingTypeToken = 0; | ||
| return false; | ||
| } | ||
| else | ||
| { | ||
| enclosingTypeToken = MetadataTokens.GetToken(declaringTypeHandle); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| public bool TryGetMethodInfo(int methodDefinitionToken, [NotNullWhen(true)] out string methodName, out int declaringTypeToken) | ||
| { | ||
| var handle = (MethodDefinitionHandle)MetadataTokens.Handle(methodDefinitionToken); | ||
| if (handle.IsNil) | ||
| { | ||
| methodName = null; | ||
| declaringTypeToken = 0; | ||
| return false; | ||
| } | ||
|
|
||
| MethodDefinition methodDefinition = _reader.GetMethodDefinition(handle); | ||
| methodName = _reader.GetString(methodDefinition.Name); | ||
| declaringTypeToken = MetadataTokens.GetToken(methodDefinition.GetDeclaringType()); | ||
| return true; | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in other checks, the absence of a PDB results in an error level notification, not a 'not applicable' message. 'Not applicable' is not the right return value here, as this return value means 'the binary isn't a valid scan target'. that's not true here, managed code is a valid scan target, the problem is that we can't analyze it due to a missing pdb. you should go look and see how the native pdb reading errors handle a missing pdb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you validate again? pushed new changes. Thank you
In reply to: 563214549 [](ancestors = 563214549)