Skip to content

Commit

Permalink
Improved reporting for PDB parsing.
Browse files Browse the repository at this point in the history
  • Loading branch information
@michaelcfanning
michaelcfanning committed Feb 22, 2021
1 parent 6e10dea commit 2800141
Show file tree
Hide file tree
Showing 14 changed files with 337 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/BinSkim.Driver/AnalyzeOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class AnalyzeOptions : AnalyzeOptionsBase
[Option(
"trace",
Separator = ';',
Default = new Traces[] { },
Default = null,
HelpText = "Execution traces, expressed as a semicolon-delimited list, that " +
"should be emitted to the console and log file (if appropriate). " +
"Valid values: PdbLoad.")]
Expand Down
6 changes: 4 additions & 2 deletions src/BinSkim.Rules/PERules/BA2004.EnableSecureSourceCodeHashing.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ public override AnalysisApplicability CanAnalyzePE(PEBinary target, Sarif.Proper

public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext context)
{
if (context.PEBinary().PE.IsManaged)
PE pe = context.PEBinary().PE;

if (pe.IsManaged && !pe.IsMixedMode)
{
AnalyzeManagedAssemblyAndPdb(context);
return;
Expand Down Expand Up @@ -150,7 +152,7 @@ public void AnalyzeNativeBinaryAndPdb(BinaryAnalyzerContext context)

private void GenerateCompilandsAndLog(BinaryAnalyzerContext context, List<ObjectModuleDetails> compilandsWithOneOrMoreInsecureFileHashes, FailureLevel failureLevel)
{
string compilands = compilandsWithOneOrMoreInsecureFileHashes.CreateOutputCoalescedByLibrary();
string compilands = compilandsWithOneOrMoreInsecureFileHashes.CreateOutputCoalescedByCompiler();

//'{0}' is a native binary that links one or more object files which were hashed
// using an insecure checksum algorithm (MD5). MD5 is subject to collision attacks
Expand Down
176 changes: 151 additions & 25 deletions src/BinSkim.Rules/PERules/BA2006.BuildWithSecureTools.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics;
using System.Globalization;

using Microsoft.CodeAnalysis.BinaryParsers;
Expand Down Expand Up @@ -36,8 +37,10 @@ public class BuildWithSecureTools : WindowsBinaryAndPdbSkimmerBase, IOptionsProv

protected override IEnumerable<string> MessageResourceNames => new string[] {
nameof(RuleResources.BA2006_Error),
nameof(RuleResources.BA2006_Error_OutdatedCsc),
nameof(RuleResources.BA2006_Error_BadModule),
nameof(RuleResources.BA2006_Pass),
nameof(RuleResources.BA2006_Pass_Csc),
nameof(RuleResources.NotApplicable_InvalidMetadata)};

public IEnumerable<IOption> GetOptions()
Expand All @@ -52,7 +55,6 @@ public IEnumerable<IOption> GetOptions()

private const string AnalyzerName = RuleIds.BuildWithSecureTools + "." + nameof(BuildWithSecureTools);

private const string MIN_COMPILER_VER = "MinimumCompilerVersion";
private const string MIN_XBOX_COMPILER_VER = "MinimumXboxCompilerVersion";

public static PerLanguageOption<StringToVersionMap> MinimumToolVersions { get; } =
Expand All @@ -75,9 +77,6 @@ public override AnalysisApplicability CanAnalyzePE(PEBinary target, PropertiesDi
reasonForNotAnalyzing = MetadataConditions.ImageIsILOnlyAssembly;
if (portableExecutable.IsILOnly) { return result; }

reasonForNotAnalyzing = MetadataConditions.ImageIsResourceOnlyBinary;
if (portableExecutable.IsResourceOnly) { return result; }

reasonForNotAnalyzing = null;
return AnalysisApplicability.ApplicableToSpecifiedTarget;
}
Expand All @@ -87,25 +86,74 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
PEBinary target = context.PEBinary();
Pdb pdb = target.Pdb;

Version minCompilerVersion;
if (target.PE.IsManaged && !target.PE.IsMixedMode)
{
AnalyzeManagedPE(context);
}

minCompilerVersion = (target.PE.IsXBox)
? context.Policy.GetProperty(MinimumToolVersions)[MIN_XBOX_COMPILER_VER]
: context.Policy.GetProperty(MinimumToolVersions)[MIN_COMPILER_VER];
Version minCompilerVersion = new Version(int.MaxValue, int.MaxValue);

var badModuleList = new TruncatedCompilandRecordList();
var badModules = new List<ObjectModuleDetails>();
StringToVersionMap allowedLibraries = context.Policy.GetProperty(AllowedLibraries);

var languageToBadModules = new Dictionary<Language, List<ObjectModuleDetails>>();

foreach (DisposableEnumerableView<Symbol> omView in pdb.CreateObjectModuleIterator())
{
Symbol om = omView.Value;
ObjectModuleDetails omDetails = om.GetObjectModuleDetails();

if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftNativeCompiler)
switch (omDetails.Language)
{
continue;
case Language.LINK:
{
continue;
}

case Language.C:
case Language.Cxx:
{
minCompilerVersion = (target.PE.IsXBox)
? context.Policy.GetProperty(MinimumToolVersions)[MIN_XBOX_COMPILER_VER]
: context.Policy.GetProperty(MinimumToolVersions)[nameof(Language.C)];
break;
}

case Language.MASM:
{
minCompilerVersion =
context.Policy.GetProperty(MinimumToolVersions)[nameof(Language.MASM)];
break;
}

case Language.CVTRES:
{
minCompilerVersion =
context.Policy.GetProperty(MinimumToolVersions)[nameof(Language.CVTRES)];
break;
}

case Language.CSharp:
{
minCompilerVersion =
context.Policy.GetProperty(MinimumToolVersions)[nameof(Language.CSharp)];
break;
}

case Language.Unknown:
{
minCompilerVersion =
context.Policy.GetProperty(MinimumToolVersions)[nameof(Language.Unknown)];
break;
}

default:
{
continue;
}
}


// See if the item is in our skip list
if (!string.IsNullOrEmpty(om.Lib))
{
Expand All @@ -119,15 +167,24 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
}

Version actualVersion;
Version minimumVersion;
Version minimumVersion = minCompilerVersion;
Language omLanguage = omDetails.Language;
switch (omLanguage)
{
case Language.C:
case Language.Cxx:
{
actualVersion = Minimum(omDetails.CompilerBackEndVersion, omDetails.CompilerFrontEndVersion);
minimumVersion = minCompilerVersion;
break;
}

case Language.LINK:
case Language.MASM:
case Language.CVTRES:
{
actualVersion = omDetails.CompilerBackEndVersion;
break;
}

default:
continue;
Expand Down Expand Up @@ -169,19 +226,17 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte

if (foundIssue)
{
// built with {0} compiler version {1} (Front end version: {2})
badModuleList.Add(
om.CreateCompilandRecordWithSuffix(
string.Format(CultureInfo.InvariantCulture,
RuleResources.BA2006_Error_BadModule,
omLanguage, omDetails.CompilerBackEndVersion, omDetails.CompilerFrontEndVersion)));
badModules.Add(omDetails);
}
}

if (!badModuleList.Empty)
if (badModules.Count != 0)
{
string badModulesText = badModules.CreateOutputCoalescedByCompiler();
string minimumRequiredCompilers = BuildMinimumCompilersList(context, languageToBadModules);

// '{0}' was compiled with one or more modules which were not built using
// minimum required tool versions (compiler version {1}). More recent toolchains
// minimum required tool versions ({1}). More recent toolchains
// contain mitigations that make it more difficult for an attacker to exploit
// vulnerabilities in programs they produce. To resolve this issue, compile
// and /or link your binary with more recent tools. If you are servicing a
Expand All @@ -192,8 +247,8 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
RuleUtilities.BuildResult(FailureLevel.Error, context, null,
nameof(RuleResources.BA2006_Error),
context.TargetUri.GetFileName(),
minCompilerVersion.ToString(),
badModuleList.CreateSortedObjectList()));
minimumRequiredCompilers,
badModulesText));
return;
}

Expand All @@ -206,6 +261,62 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
minCompilerVersion.ToString()));
}

private string BuildMinimumCompilersList(BinaryAnalyzerContext context, Dictionary<Language, List<ObjectModuleDetails>> languageToBadModules)
{
var languages = new List<string>();

foreach(Language language in languageToBadModules.Keys)
{
Version version = context.Policy.GetProperty(MinimumToolVersions)[language.ToString()];
languages.Add($"{language} ({version})");
}
return string.Join(", ", languages);
}

private string BuildBadModulesList(Dictionary<Language, List<ObjectModuleDetails>> languageToBadModules)
{
var coalescedModules = new List<string>();

foreach (Language language in languageToBadModules.Keys)
{
string modulesText = languageToBadModules[language].CreateOutputCoalescedByCompiler();
coalescedModules.Add(modulesText);
}
return string.Join(string.Empty, coalescedModules);
}


private void AnalyzeManagedPE(BinaryAnalyzerContext context)
{
Version minCscVersion =
context.Policy.GetProperty(MinimumToolVersions)[nameof(Language.CSharp)];

PE pe = context.PEBinary().PE;

if (pe.LinkerVersion < minCscVersion)
{
// '{0}' is a managed assembly that was compiled with an outdated toolchain
// ({1}) that does not support security features (such as SHA256 PDB
// checksums and reproducible builds) that must be enabled by policy. To
// resolve this issue, compile with more recent tools ({2} or later).
context.Logger.Log(this,
RuleUtilities.BuildResult(FailureLevel.Error, context, null,
nameof(RuleResources.BA2006_Error_OutdatedCsc),
context.TargetUri.GetFileName(),
pe.LinkerVersion.ToString(),
minCscVersion.ToString()));

return;
}

// '{0}' is a managed assembly that was compiled with toolchain ({1}) that supports all security features that must be enabled by policy.
context.Logger.Log(this,
RuleUtilities.BuildResult(ResultKind.Pass, context, null,
nameof(RuleResources.BA2006_Pass_Csc),
context.TargetUri.GetFileName(),
pe.LinkerVersion.ToString()));
}

public static Version Minimum(Version lhs, Version rhs)
{
return (lhs < rhs) ? lhs : rhs;
Expand All @@ -215,10 +326,25 @@ private static StringToVersionMap BuildMinimumToolVersionsMap()
{
var result = new StringToVersionMap
{
[MIN_COMPILER_VER] = new Version(17, 0, 65501, 17013),
[nameof(Language.C)] = new Version(17, 0, 65501, 17013),
[nameof(Language.Cxx)] = new Version(17, 0, 65501, 17013),
[nameof(Language.MASM)] = new Version(14, 15, 26706, 0),
[nameof(Language.LINK)] = new Version(17, 0, 65501, 17013),
[nameof(Language.CSharp)] = new Version(19, 0, 0, 0),
[nameof(Language.CVTRES)] = new Version(14, 14, 26430, 0),
[nameof(Language.Unknown)] = new Version(int.MaxValue, int.MaxValue, int.MaxValue, int.MaxValue),
[MIN_XBOX_COMPILER_VER] = new Version(16, 0, 11886, 0)
};

foreach (string name in Enum.GetNames(typeof(Language)))
{
if (!result.ContainsKey(name))
{
// If we don't have entry for a language, fire on everything.
result[name] = new Version(int.MaxValue, int.MaxValue);
}
}

return result;
}

Expand All @@ -227,7 +353,7 @@ private static StringToVersionMap BuildAllowedLibraries()
var result = new StringToVersionMap();

// Example entries
// result["cExample.lib,c"] = new Version("1.0.0.0")
result["libeay32.lib,unknown"] = new Version("0.0.0.0");
// result["cplusplusExample.lib,cxx"] = new Version("1.0.0.0")
// result["masmExample.lib,masm"] = new Version("1.0.0.0")

Expand Down
2 changes: 1 addition & 1 deletion src/BinSkim.Rules/PERules/BA2007.EnableCriticalCompilerWarnings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
ObjectModuleDetails omDetails = om.GetObjectModuleDetails();

// Detection applies to C/C++ produced by MS compiler only
if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftNativeCompiler)
if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftC)
{
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/BinSkim.Rules/PERules/BA2011.EnableStackProtection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
ObjectModuleDetails omDetails = om.GetObjectModuleDetails();

// Detection applies to C/C++ produced by MS compiler only
if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftNativeCompiler)
if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftC)
{
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/BinSkim.Rules/PERules/BA2024.EnableSpectreMitigations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext conte
case Language.C:
case Language.Cxx:
{
if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftNativeCompiler)
if (omDetails.WellKnownCompiler != WellKnownCompilers.MicrosoftC)
{
// TODO: https://github.com/Microsoft/binskim/issues/114
continue;
Expand Down
18 changes: 15 additions & 3 deletions src/BinSkim.Rules/PERules/WindowsBinaryAndPdbSkimmerBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ namespace Microsoft.CodeAnalysis.IL.Rules
// Windows specific binary and program database-reading skimmers.
public abstract class WindowsBinaryAndPdbSkimmerBase : WindowsBinarySkimmerBase
{
/// <summary>
/// Gets a property indicating whether the rule should require that PDBs
/// can be located for managed assemblies. Some checks that inspect both
/// managed and native code require PDBs for the native case but not
/// for managed.
/// </summary>
public virtual bool EnforcePdbLoadForManagedAssemblies => true;

public sealed override void Analyze(BinaryAnalyzerContext context)
{
// Uses PDB Parsing.
Expand All @@ -32,7 +40,10 @@ public sealed override void Analyze(BinaryAnalyzerContext context)
target.Pdb.LoadTrace = null;
}

if (target.Pdb == null)
if ((!target.PE.IsManaged ||
target.PE.IsMixedMode ||
EnforcePdbLoadForManagedAssemblies)
&& target.Pdb == null)
{
LogExceptionLoadingPdb(context, target.PdbParseException);
return;
Expand Down Expand Up @@ -104,7 +115,7 @@ public static void LogExceptionLoadingPdb(IAnalysisContext context, PdbException
throw new ArgumentNullException(nameof(context));
}

// '{0}' was not evaluated for check '{1}' because its PDB could not be loaded.
// '{0}' was not evaluated for check '{1}' because its PDB could not be loaded ({2}).
context.Logger.LogConfigurationNotification(
Errors.CreateNotification(
context.TargetUri,
Expand All @@ -115,7 +126,8 @@ public static void LogExceptionLoadingPdb(IAnalysisContext context, PdbException
persistExceptionStack: false,
RuleResources.ERR997_ExceptionLoadingPdb,
context.TargetUri.GetFileName(),
context.Rule.Name));
context.Rule.Name,
pdbException.ExceptionCode.ToString()));

context.RuntimeErrors |= RuntimeConditions.ExceptionLoadingPdb;

Expand Down
Loading

0 comments on commit 2800141

Please sign in to comment.