Throwing once per file #465

eddynaka · 2021-08-23T13:56:20Z

Fixes #304

Analyzing...
Analyzing 'Native_x64_VS2013_Default.dll'...
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2002 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'DoNotIncorporateVulnerableDependencies' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2004 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'EnableSecureSourceCodeHashing' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2006 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'BuildWithSecureTools' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2007 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'EnableCriticalCompilerWarnings' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2011 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'EnableStackProtection' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2013 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'InitializeStackProtection' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2014 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'DoNotDisableStackProtectionForFunctions' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2024 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'EnableSpectreMitigations' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2025 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'EnableShadowStack' because its PDB could not be loaded (E_PDB_NOT_FOUND).
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : BA2026 : 'Native_x64_VS2013_Default.dll' was not evaluated for check 'EnableAdditionalSdlSecurityChecks' because its PDB could not be loaded (E_PDB_NOT_FOUND).

One or more rules was disabled for an analysis target, as it was determined not to be applicable to it (this is a common condition). Pass --verbose on the command-line for more information.

Analysis did not complete due to one or more unrecoverable execution conditions.
Unexpected fatal runtime condition(s) observed: ExceptionLoadingPdb

Current:

Analyzing...
Analyzing 'Native_x64_VS2013_Default.dll'...
Native_x64_VS2013_Default.dll : error ERR997.ExceptionLoadingPdb : 'Native_x64_VS2013_Default.dll' was not evaluated because its PDB could not be loaded (E_PDB_NOT_FOUND).

One or more rules was disabled for an analysis target, as it was determined not to be applicable to it (this is a common condition). Pass --verbose on the command-line for more information.

Analysis did not complete due to one or more unrecoverable execution conditions.
Unexpected fatal runtime condition(s) observed: ExceptionLoadingPdb

eddynaka · 2021-08-23T14:02:11Z

src/BinSkim.Rules/PERules/WindowsBinaryAndPdbSkimmerBase.cs

-            // '{0}' was not evaluated for check '{1}' because its PDB could not be loaded ({2}).
+            string fileName = context.TargetUri.GetFileName();
+            string key = $"{fileName}@{pdbException.ExceptionDisplayMessage}";
+            if (s_PdbExceptions.ContainsKey(key))


s_PdbExceptions.ContainsKey(key)

If we already reported this issue for this target, we won't report twice. #ByDesign

eddynaka · 2021-08-23T14:02:40Z

src/BinSkim.Rules/PERules/WindowsBinaryAndPdbSkimmerBase.cs

            context.Logger.LogConfigurationNotification(
                Errors.CreateNotification(
                    context.TargetUri,
                    "ERR997.ExceptionLoadingPdb",
-                    context.Rule.Id,
+                    string.Empty,


string.Empty

To be generic, I removed context.Rule.Id and context.Rule.Name. #ByDesign

eddynaka · 2021-08-23T14:03:23Z

src/BinSkim.Rules/PERules/WindowsBinaryAndPdbSkimmerBase.cs

                    pdbException.ExceptionDisplayMessage));

+            s_PdbExceptions.TryAdd(key, true);


s_PdbExceptions.TryAdd

Once we log the notification, we will add to the caching variable. Since it's static and concurrent, it won't face any issue during analysis #ByDesign

shaopeng-gh · 2021-08-23T21:45:50Z

src/BinSkim.Rules/RuleResources.resx

@@ -569,4 +569,7 @@ Modules did not meet the criteria: {1}</value>
  <data name="BA5002_Pass" xml:space="preserve">
    <value>Executable stack is not allowed on executable '{0}'.</value>
  </data>
+  <data name="ERR997)ExceptionLoadingPdbGeneric" xml:space="preserve">


)

typo? #Closed

yes! thanks for reviewing!

shaopeng-gh · 2021-08-23T21:53:07Z

src/BinSkim.Rules/PERules/WindowsBinaryAndPdbSkimmerBase.cs

@@ -130,20 +134,27 @@ 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 ({2}).
+            string fileName = context.TargetUri.GetFileName();


fileName

key sould also consider path?
/x86/a.exe
/x64/a.exe #Closed

Changed to OriginalString as path. That will prevent issues like you showed.

thanks!

….com/microsoft/binskim into users/ednakamu/throwing-once-per-pdb

Throwing once per file

abe95b5

eddynaka requested a review from michaelcfanning as a code owner August 23, 2021 13:56

Merge branch 'main' into users/ednakamu/throwing-once-per-pdb

40af07d

eddynaka commented Aug 23, 2021

View reviewed changes

Adding a few improvements, adding releaseHistory to sln

19f3bb4

shaopeng-gh reviewed Aug 23, 2021

View reviewed changes

eddynaka and others added 3 commits August 23, 2021 18:53

Merge branch 'users/ednakamu/throwing-once-per-pdb' of https://github…

4881ebd

….com/microsoft/binskim into users/ednakamu/throwing-once-per-pdb

Addressing PR Feedback.

bd3cb63

Merge branch 'main' into users/ednakamu/throwing-once-per-pdb

579c8db

eddynaka merged commit b7c72ec into main Aug 24, 2021

eddynaka deleted the users/ednakamu/throwing-once-per-pdb branch August 24, 2021 13:35

shaopeng-gh mentioned this pull request May 5, 2023

Binskim not listening all check when all checks should be included #843

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throwing once per file #465

Throwing once per file #465

eddynaka commented Aug 23, 2021 •

edited

Loading

eddynaka Aug 23, 2021 •

edited

Loading

eddynaka Aug 23, 2021 •

edited

Loading

eddynaka Aug 23, 2021 •

edited

Loading

shaopeng-gh Aug 23, 2021 •

edited by eddynaka

Loading

eddynaka Aug 23, 2021

shaopeng-gh Aug 23, 2021 •

edited by eddynaka

Loading

eddynaka Aug 23, 2021

		pdbException.ExceptionDisplayMessage));

		s_PdbExceptions.TryAdd(key, true);

Throwing once per file #465

Throwing once per file #465

Conversation

eddynaka commented Aug 23, 2021 • edited Loading

eddynaka Aug 23, 2021 • edited Loading

Choose a reason for hiding this comment

eddynaka Aug 23, 2021 • edited Loading

Choose a reason for hiding this comment

eddynaka Aug 23, 2021 • edited Loading

Choose a reason for hiding this comment

shaopeng-gh Aug 23, 2021 • edited by eddynaka Loading

Choose a reason for hiding this comment

eddynaka Aug 23, 2021

Choose a reason for hiding this comment

shaopeng-gh Aug 23, 2021 • edited by eddynaka Loading

Choose a reason for hiding this comment

eddynaka Aug 23, 2021

Choose a reason for hiding this comment

eddynaka commented Aug 23, 2021 •

edited

Loading

eddynaka Aug 23, 2021 •

edited

Loading

eddynaka Aug 23, 2021 •

edited

Loading

eddynaka Aug 23, 2021 •

edited

Loading

shaopeng-gh Aug 23, 2021 •

edited by eddynaka

Loading

shaopeng-gh Aug 23, 2021 •

edited by eddynaka

Loading