Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
quasarea opened this issue Mar 22, 2023 · 7 comments
Open

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

quasarea opened this issue Mar 22, 2023 · 7 comments
Labels
4.1 Fix these issues for v4.1

Comments

@quasarea
Copy link

Calling

.\microsoft.codeanalysis.binskim.2.0.0-rc2\tools\netcoreapp3.1\win-x64\BinSkim.exe  `
  analyze C:\temp\\lib\x64\Release\library.dll `
  --level Error Warning Note `
  --kind Fail Pass Review Open NotApplicable Informational

results with:

Analyzing...
..\lib\x64\Release\library.dll: notapplicable BA2015: 'library.dll' was not evaluated for check 'EnableHighEntropyVirtualAddresses' as the analysis is not relevant based on observed metadata: image is not an executable program.
..\lib\x64\Release\library.dll: notapplicable BA2016: 'library.dll' was not evaluated for check 'MarkImageAsNXCompatible' as the analysis is not relevant based on observed metadata: image is a 64-bit binary.
..\lib\x64\Release\library.dll: notapplicable BA2018: 'library.dll' was not evaluated for check 'EnableSafeSEH' as the analysis is not relevant based on observed metadata: image is not a 32-bit binary.
..\lib\x64\Release\library.dll: notapplicable BA3001: 'library.dll' was not evaluated for check 'EnablePositionIndependentExecutable' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3002: 'library.dll' was not evaluated for check 'DoNotMarkStackAsExecutable' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3003: 'library.dll' was not evaluated for check 'EnableStackProtector' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3004: 'library.dll' was not evaluated for check 'GenerateRequiredSymbolFormat' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3005: 'library.dll' was not evaluated for check 'EnableStackClashProtection' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3006: 'library.dll' was not evaluated for check 'EnableNonExecutableStack' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3010: 'library.dll' was not evaluated for check 'EnableReadOnlyRelocations' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3011: 'library.dll' was not evaluated for check 'EnableBindNow' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3030: 'library.dll' was not evaluated for check 'UseGccCheckedFunctions' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA3031: 'library.dll' was not evaluated for check 'EnableClangSafeStack' as the analysis is not relevant based on observed metadata: image is not an ELF binary.
..\lib\x64\Release\library.dll: notapplicable BA5001: 'library.dll' was not evaluated for check 'EnablePositionIndependentExecutableMachO' as the analysis is not relevant based on observed metadata: image is not a MachO binary.
..\lib\x64\Release\library.dll: notapplicable BA5002: 'library.dll' was not evaluated for check 'DoNotAllowExecutableStack' as the analysis is not relevant based on observed metadata: image is not a MachO binary.
..\lib\x64\Release\library.dll: notapplicable BA6001: 'library.dll' was not evaluated for check 'DisableIncrementalLinkingInReleaseBuilds' as the analysis is not relevant based on observed metadata: an exception occurred attempting to load its pdb.
..\lib\x64\Release\library.dll: notapplicable BA6002: 'library.dll' was not evaluated for check 'EliminateDuplicateStrings' as the analysis is not relevant based on observed metadata: an exception occurred attempting to load its pdb.
..\lib\x64\Release\library.dll: notapplicable BA6004: 'library.dll' was not evaluated for check 'EnableComdatFolding' as the analysis is not relevant based on observed metadata: an exception occurred attempting to load its pdb.
..\lib\x64\Release\library.dll: notapplicable BA6005: 'library.dll' was not evaluated for check 'EnableOptimizeReferences' as the analysis is not relevant based on observed metadata: an exception occurred attempting to load its pdb.
..\lib\x64\Release\library.dll: notapplicable BA6006: 'library.dll' was not evaluated for check 'EnableLinkTimeCodeGeneration' as the analysis is not relevant based on observed metadata: an exception occurred attempting to load its pdb.
..\lib\x64\Release\library.dll: pass BA2001: 'library.dll' is a 64-bit image with a base address that is >= 4 gigabytes, increasing the effectiveness of Address Space Layout Randomization (which helps prevent attackers from executing security-sensitive code in well-known locations).
..\lib\x64\Release\library.dll: pass BA2005: 'library.dll' is not known to be an obsolete binary that is vulnerable to one or more security problems.
..\lib\x64\Release\library.dll: error BA2008: 'library.dll' does not enable the control flow guard (CFG) mitigation. To resolve this issue, pass /guard:cf on both the compiler and linker command lines. Binaries also require the /DYNAMICBASE linker option in order to enable CFG.
..\lib\x64\Release\library.dll: error BA2009: 'library.dll' is not marked as DYNAMICBASE. This means that the binary is not eligible for relocation by Address Space Layout Randomization (ASLR). ASLR is an important mitigation that makes it more difficult for an attacker to exploit memory corruption vulnerabilities. To resolve this issue, configure your tools to build with this feature enabled. For C and C++ binaries, add /DYNAMICBASE to your linker command line. For .NET applications, use a compiler shipping with Visual Studio 2008 or later.
..\lib\x64\Release\library.dll: pass BA2010: 'library.dll' does not have an imports section that is marked as executable, helping to prevent the exploitation of code vulnerabilities.
..\lib\x64\Release\library.dll: pass BA2012: 'library.dll' is a C or C++ binary built with the buffer security feature that properly preserves the stack protecter cookie. This has the effect of enabling a significant increase in entropy provided by the operating system over that produced by the C runtime start-up code.
..\lib\x64\Release\library.dll: pass BA2019: 'library.dll' contains no data or code sections marked as both shared and writable, helping to prevent the exploitation of code vulnerabilities.
..\lib\x64\Release\library.dll: pass BA2021: 'library.dll' contains no data or code sections marked as both shared and executable, helping to prevent the exploitation of code vulnerabilities.
..\lib\x64\Release\library.dll: pass BA2022: 'library.dll' appears to be signed with secure cryptographic algorithms. WinTrustVerify successfully validated the binary but did not attempt to validate certificate chaining or that the root certificate is trusted. The following digitial signature algorithms were detected: Cryptographically strong signatures: [digest algorithm: 'sha256NoSign' + digest encryption algorithm: 'RSA']
..\lib\x64\Release\library.dll : error ERR997.ExceptionLoadingPdb : 'library.dll' was not evaluated because its PDB could not be loaded (E_PDB_NOT_FOUND).

Done. 1 files scanned.

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

there is no mention of number of tests, i.e. BA2011, while in past it was mentioned, in example here: Azure/azure-cosmos-dotnet-v3#2821 where you clearly see what checks were skipped due to missing pdb:

~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2002 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'DoNotIncorporateVulnerableDependencies' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2006 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'BuildWithSecureTools' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2007 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'EnableCriticalCompilerWarnings' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2011 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'EnableStackProtection' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2013 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'InitializeStackProtection' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2014 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'DoNotDisableStackProtectionForFunctions' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
~\path\Cosmos.CRTCompat.dll : error ERR997.ExceptionLoadingPdb : BA2024 : 'Cosmos.CRTCompat.dll' was not evaluated for check 'EnableSpectreMitigations' because its PDB could not be loaded. (E_PDB_NOT_FOUND (File not found))
@michaelcfanning michaelcfanning added the 4.1 Fix these issues for v4.1 label Apr 5, 2023
@michaelcfanning
Copy link
Member

We should investigate this for 4.0.1, our next release.

@suvamM
Copy link
Collaborator

suvamM commented Apr 27, 2023

@quasarea I am looking into this issue and trying to understand it better. From the outputs above, it does not seem like there is a problem: there is an aggregated error ER997.ExceptionLoadingPdb while running the analysis on the library.dll, instead of an ER997 error for every rule, as you showed in the output below it. I think this is by design. Could you please confirm if this is the problem you are reporting?

@quasarea
Copy link
Author

quasarea commented Apr 27, 2023

@suvamM I was not aware that ER997.ExceptionLoadingPdb is aggregated, is there documentation entry for what it aggregates? I need to convert this output to matrix, so need to know what fields I need to fill in. Personally would prefer an option to not aggregate such errors, are those showing in sarif files correctly? Will check tomorrow

@suvamM
Copy link
Collaborator

suvamM commented Apr 27, 2023

@suvamM I was not aware that ER997.ExceptionLoadingPdb is agregated, is there documentation entry for what it agregates? I need to convert this output to matrix, so need to know what fields I need to fill in. Personally would prefer an option to not agregate such errors, are those showing in sarif files correctly? Will check tomorrow

OK, so I understood your problem correctly :)
Let me check the aggregation logic.

@quasarea
Copy link
Author

I can confirm that sarif does not contain information about particular tests as well, just aggregation. I think sarif should contain complete information instead. I could add script that if ERR997.ExceptionLoadingPdb then BA2002, BA2006, BA2007, BA2011, BA2013, BA2014, BA2024 failed, but I will have to keep track on your documentation so when you add another test that depends on pdb, I will extend my script. It is not perfect solutions for me ;)

@shaopeng-gh
Copy link
Collaborator

thanks for reporting, adding my input,
This was actually implemented as a breaking change by request:
#465
the binary lacking of pdb is a single issue and can be fixed by a single action to add the missing pdb, and I believe most generic users of BinSkim as a tool would prefer not have the issue duplicated.
This change however as a breaking change, will be inconvenient for advanced user that is looking for a complete list.

This looks like a by design for me.

@quasarea
Copy link
Author

quasarea commented May 7, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.1 Fix these issues for v4.1
Projects
None yet
Development

No branches or pull requests

4 participants