diff --git a/ReleaseHistory.md b/ReleaseHistory.md index 1d0562380..5de4ab141 100644 --- a/ReleaseHistory.md +++ b/ReleaseHistory.md @@ -16,6 +16,7 @@ - NEW => new feature ## UNRELEASED * DEP: Update `Sarif.Sdk` submodule from [bc8cb57 to fd6e615](https://github.com/microsoft/sarif-sdk/compare/bc8cb57...fd6e615). Reference [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/fd6e615/ReleaseHistory.md). +* BUG: Fix `BA2027.EnableSourceLink` unexpectedly causes `ExceptionLoadingPdb` error when the PDB file is missing. [988](https://github.com/microsoft/binskim/pull/988). * BUG: Exclude system-generated files `AssemblyAttributes.obj`, `AssemblyInfo.obj`, `stdafx.obj` from `BA2004.EnableSecureSourceCodeHashing`. [989](https://github.com/microsoft/binskim/pull/989). * BUG: Fix `ERR998.ExceptionInAnalyze`: `InvalidOperationException: Unrecognized crypto HRESULT: 0x80096011` for check `BA2022.SignSecurely` when the signature is malformed, by adding missing error code to error description mappings. [969](https://github.com/microsoft/binskim/pull/969) * NEW: `BA4002.ReportElfOrMachoCompilerData`, which collects telemetry data for Elf and Macho files, is now enabled by default. diff --git a/src/BinSkim.Rules/PERules/BA2027.EnableSourceLink.cs b/src/BinSkim.Rules/PERules/BA2027.EnableSourceLink.cs index 20a72f868..dd8a0fe47 100644 --- a/src/BinSkim.Rules/PERules/BA2027.EnableSourceLink.cs +++ b/src/BinSkim.Rules/PERules/BA2027.EnableSourceLink.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System; using System.Collections.Generic; using System.Composition; using System.Linq; @@ -22,6 +21,8 @@ public class EnableSourceLink : WindowsBinaryAndPdbSkimmerBase /// public override string Id => RuleIds.EnableSourceLink; + public override bool LogPdbLoadException => false; + /// /// Enable SourceLink. /// @@ -66,6 +67,14 @@ public override AnalysisApplicability CanAnalyzePE(PEBinary target, BinaryAnalyz public override void AnalyzePortableExecutableAndPdb(BinaryAnalyzerContext context) { + PEBinary target = context.PEBinary(); + Pdb pdb = target.Pdb; + + if (pdb == null) + { + return; + } + if (!HasSourceLink(context)) { // The PDB for '{0}' does not contain SourceLink information, compromising diff --git a/src/Test.FunctionalTests.BinSkim.Driver/AnalyzeCommandTests.cs b/src/Test.FunctionalTests.BinSkim.Driver/AnalyzeCommandTests.cs index 3896e3d13..d10a2f6cc 100644 --- a/src/Test.FunctionalTests.BinSkim.Driver/AnalyzeCommandTests.cs +++ b/src/Test.FunctionalTests.BinSkim.Driver/AnalyzeCommandTests.cs @@ -111,7 +111,7 @@ public void AnalyzeCommand_DeterminismTest() WindowsBinaryAndPdbSkimmerBase.s_PdbExceptions.Clear(); string fileName = Path.Combine(Path.GetTempPath(), "AnalyzeCommand_DeterminismTest.sarif"); - string pathDeterminismTest = Path.Combine(PEBinaryTests.TestData, "PE", "Determinism", "*.dll"); + string pathDeterminismTest = Path.Combine(PEBinaryTests.TestData, "PE", "Determinism", "*.exe"); var options = new AnalyzeOptions { TargetFileSpecifiers = new string[] { diff --git a/src/Test.UnitTests.BinSkim.Rules/RulePropertyTests.cs b/src/Test.UnitTests.BinSkim.Rules/RulePropertyTests.cs new file mode 100644 index 000000000..aadc5677c --- /dev/null +++ b/src/Test.UnitTests.BinSkim.Rules/RulePropertyTests.cs @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; + +using FluentAssertions; + +using Xunit; + +namespace Microsoft.CodeAnalysis.IL.Rules +{ + public class RulePropertyTests + { + private static readonly string[] ExpectedLogPdbLoadExceptionRules = new string[] + { + "BA2002.DoNotIncorporateVulnerableDependencies", + "BA2006.BuildWithSecureTools", + "BA2007.EnableCriticalCompilerWarnings", + "BA2011.EnableStackProtection", + "BA2013.InitializeStackProtection", + "BA2014.DoNotDisableStackProtectionForFunctions", + "BA2024.EnableSpectreMitigations", + "BA2025.EnableShadowStack", + "BA2026.EnableMicrosoftCompilerSdlSwitch", + "BA6001.DisableIncrementalLinkingInReleaseBuilds", + "BA6002.EliminateDuplicateStrings", + "BA6004.EnableComdatFolding", + "BA6005.EnableOptimizeReferences", + "BA6006.EnableLinkTimeCodeGeneration" + }; + + [Fact] + public void RulePropertyTests_LogPdbLoadException() + { + WindowsBinaryAndPdbSkimmerBase[] skimmers = + GetAllWindowsBinaryAndPdbSkimmers("BinSkim.Rules.dll"); + IEnumerable actualLogPdbLoadExceptionRules = + skimmers.Where(s => s.LogPdbLoadException); + IEnumerable unexpectedLogPdbLoadExceptionRules = + actualLogPdbLoadExceptionRules.Where(s => !ExpectedLogPdbLoadExceptionRules.Contains(s.Moniker)); + + if (unexpectedLogPdbLoadExceptionRules.Any()) + { + Assert.Fail(string.Format("Please examine if the following rules should enable 'LogPdbLoadException': {0}", + string.Join(", ", unexpectedLogPdbLoadExceptionRules.Select(skimmer => skimmer.Moniker)))); + } + } + + private static WindowsBinaryAndPdbSkimmerBase[] GetAllWindowsBinaryAndPdbSkimmers(string rulesAssemblyName) + { + string directory = AppDomain.CurrentDomain.BaseDirectory; + string assemblyPath = Path.Combine(directory, rulesAssemblyName); + var assembly = Assembly.LoadFrom(assemblyPath); + Type[] assemblyTypes = assembly.GetTypes(); + IEnumerable inheritanceTypes = + assemblyTypes.Where(t => t.BaseType == typeof(WindowsBinaryAndPdbSkimmerBase)); + IEnumerable instances = + inheritanceTypes.Select(t => (WindowsBinaryAndPdbSkimmerBase)Activator.CreateInstance(t)); + return instances.ToArray(); + } + } +} diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/CPlusPlus_SourceLink.exe b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/CPlusPlus_SourceLink.exe new file mode 100644 index 000000000..052beaede Binary files /dev/null and b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/CPlusPlus_SourceLink.exe differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/Managed_x64_VS2022_CSharp_Net70_Default.pdb b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/CPlusPlus_SourceLink.pdb similarity index 100% rename from src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/Managed_x64_VS2022_CSharp_Net70_Default.pdb rename to src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/CPlusPlus_SourceLink.pdb diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/Managed_x64_VS2022_CSharp_Net70_Default.dll b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/Managed_x64_VS2022_CSharp_Net70_Default.dll deleted file mode 100644 index 24d190d9e..000000000 Binary files a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_OUTOFMEMORY/Managed_x64_VS2022_CSharp_Net70_Default.dll and /dev/null differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/CPlusPlus_SourceLink.exe b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/CPlusPlus_SourceLink.exe new file mode 100644 index 000000000..052beaede Binary files /dev/null and b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/CPlusPlus_SourceLink.exe differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/Managed_x64_VS2022_CSharp_Net70_Default.pdb b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/CPlusPlus_SourceLink.pdb similarity index 100% rename from src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/Managed_x64_VS2022_CSharp_Net70_Default.pdb rename to src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/CPlusPlus_SourceLink.pdb diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/Managed_x64_VS2022_CSharp_Net70_Default.dll b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/Managed_x64_VS2022_CSharp_Net70_Default.dll deleted file mode 100644 index 24d190d9e..000000000 Binary files a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_FORMAT/Managed_x64_VS2022_CSharp_Net70_Default.dll and /dev/null differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_NOT_FOUND/CPlusPlus_SourceLink.exe b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_NOT_FOUND/CPlusPlus_SourceLink.exe new file mode 100644 index 000000000..052beaede Binary files /dev/null and b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_NOT_FOUND/CPlusPlus_SourceLink.exe differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_NOT_FOUND/Managed_x64_VS2022_CSharp_Net70_Default.dll b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_NOT_FOUND/Managed_x64_VS2022_CSharp_Net70_Default.dll deleted file mode 100644 index 24d190d9e..000000000 Binary files a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/E_PDB_NOT_FOUND/Managed_x64_VS2022_CSharp_Net70_Default.dll and /dev/null differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/CPlusPlus_SourceLink.exe b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/CPlusPlus_SourceLink.exe new file mode 100644 index 000000000..052beaede Binary files /dev/null and b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/CPlusPlus_SourceLink.exe differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/CPlusPlus_SourceLink.pdb b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/CPlusPlus_SourceLink.pdb new file mode 100644 index 000000000..56d4c70e5 Binary files /dev/null and b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/CPlusPlus_SourceLink.pdb differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/Managed_x64_VS2022_CSharp_Net70_Default.dll b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/Managed_x64_VS2022_CSharp_Net70_Default.dll deleted file mode 100644 index 24d190d9e..000000000 Binary files a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/Managed_x64_VS2022_CSharp_Net70_Default.dll and /dev/null differ diff --git a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/Managed_x64_VS2022_CSharp_Net70_Default.pdb b/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/Managed_x64_VS2022_CSharp_Net70_Default.pdb deleted file mode 100644 index 0c96f8f9a..000000000 Binary files a/src/Test.UnitTests.BinaryParsers/TestData/PE/Determinism/OK/Managed_x64_VS2022_CSharp_Net70_Default.pdb and /dev/null differ