search

microsoft / binskim

A binary static analysis tool that provides security and correctness results for Windows Portable Executable and *nix ELF binary formats
Other
775 stars 158 forks source link

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

Open quasarea opened 1 year ago

quasarea commented 1 year ago

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: https://github.com/Azure/azure-cosmos-dotnet-v3/issues/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 commented 1 year ago

We should investigate this for 4.0.1, our next release.

suvamM commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

thanks for reporting, adding my input, This was actually implemented as a breaking change by request: https://github.com/microsoft/binskim/pull/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 commented 1 year ago

Its all fine if you have documented what tests are aggregated into that error. Personally I dont think it should behave like that in sarif output.

Regards, J

On Fri, 5 May 2023, 19:14 Shaopeng, @.***> wrote:

thanks for reporting, adding my input, This was actually implemented as a breaking change by request:

465 https://github.com/microsoft/binskim/pull/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.

— Reply to this email directly, view it on GitHub https://github.com/microsoft/binskim/issues/843#issuecomment-1536608373, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACO3GBX43VZB7GWAPJE47C3XEU7PZANCNFSM6AAAAAAWDYGLRM . You are receiving this because you were mentioned.Message ID: @.***>