Exclude ARM Binaries from BA2025 analysis #650

marmegh · 2022-06-01T20:57:03Z

Addressing issue #627 by:

Exclude ARM binaries from BA2025 analysis
Added ARM not applicable test case (.exe and .pdb files)
Extended test functionality to confirm correct skip reason is present. This allows confirmation for the current BA2025 test cases and can be used to add granularity to existing/future test cases.

marmegh · 2022-06-01T20:58:36Z

src/BinSkim.Rules/PERules/BA2025.EnableShadowStack.cs

+            {
+                reasonForNotAnalyzing = MetadataConditions.ImageIsArmBinary;
+                return notApplicable;
+            }


This is the only logic update for this change. Specifically, excluding ARM binaries from BA2025 analysis in a similar fashion as ARM64 binaries.

Nice code comment! Absolutely the info I was looking for. :)

marmegh · 2022-06-01T20:59:40Z

src/BinSkim.Sdk/MetadataConditions.cs

@@ -10,6 +10,7 @@ public static class MetadataConditions
        public static readonly string ImageIsNotExe = SdkResources.MetadataCondition_ImageIsNotExe;
        public static readonly string ImageIsNotMachO = SdkResources.MetadataCondition_ImageIsNotMachO;
        public static readonly string CouldNotLoadPdb = SdkResources.MetadataCondition_CouldNotLoadPdb;
+        public static readonly string ImageIsArmBinary = SdkResources.MetadataCondition_ImageIsArmBinary;


public static readonly string ImageIsArmBinary = SdkResources.MetadataCondition_ImageIsArmBinary;

Add a MetadataConditions entry for ARMBinaries. This is separate as changing the previous user facing string could confuse customers by changing details in the results.

marmegh · 2022-06-01T21:20:47Z

src/Test.FunctionalTests.BinSkim.Rules/RuleTests.cs

+            AnalysisApplicability expectedApplicability,
+            string ruleName,
+            string expectedReasonForNotAnalyzing)
+        {


Created method for shared code.

shaopeng-gh · 2022-06-10T21:25:47Z

src/Test.FunctionalTests.BinSkim.Rules/RuleTests.cs

@@ -460,6 +498,17 @@ private HashSet<string> GetTestFilesMatchingConditions(HashSet<string> metadataC
                result.Add(Path.Combine(testFilesDirectory, "DotnetNative_x86_VS2019_UniversalApp.exe"));
            }

+            if (metadataConditions.Contains(MetadataConditions.ImageIsArmBinary))
+            {
+                result.Add(Path.Combine(testFilesDirectory, "ARM_CETShadowStack_NotApplicable.exe"));


ARM_CETShadowStack_NotApplicable.exe

nice, how did you generate this file, please add the step in md file
\docs\FunctionalTestBuildScripts.md #Closed

shaopeng-gh · 2022-06-10T21:30:37Z

src/Test.FunctionalTests.BinSkim.Rules/RuleTests.cs

+            string expectedReasonForNotAnalyzing)
+        {
+
+            var context = new BinaryAnalyzerContext();


first line empty can be removed. #Closed

shaopeng-gh

🕐

shaopeng-gh · 2022-06-10T21:56:59Z

src/ReleaseHistory.md

@@ -7,6 +7,7 @@
 * Bump ELFSharp from 2.14.0 to 2.15.0. [#631](https://github.com/microsoft/binskim/pull/631)
 * FEATURE: Enable BinSkim for MacOS. [#576](https://github.com/microsoft/binskim/pull/576)
 * Bump Sarif.Sdk by updating submodule from [4e9f606 to fc9a9df](https://github.com/microsoft/sarif-sdk/compare/4e9f606bb0e88428866e253352cdc70dc68f98cb...fc9a9dfb865096b5aaa9fa3651854670940f7459). [#638](https://github.com/microsoft/binskim/pull/638)
+* BUGFIX: Fix `BA2025.EnableShadowStack` warning for ARM Binaries which cannot use `/CETCOMPAT`. [#650](https://github.com/microsoft/binskim/pull/650)


BUGFIX: Fix BA2025.EnableShadowStack warning for ARM Binaries which cannot use /CETCOMPAT.

I see a sample below:
FALSE POSITIVE FIX: Skip PDB-driven analysis for the generated .NET core native bootstrap exe (which is not user-controllable code).
See if we think this template is better for this change
FALSE POSITIVE FIX: Skip rule xxx for xxx (reason) #Closed

…naries.

shaopeng-gh · 2022-06-11T00:03:25Z

docs/FunctionalTestBuildScripts.md

@@ -47,3 +47,40 @@ Also create two user functions `userfn_use_safebuffers_1()` and `userfn_use_safe
 A simple `Windows Kernel Mode Driver` program, created with `Visual Studio 2019` that generates a .exe and associated .pdb file. Code to reproduce:  
 Use `NTSTATUS GsDriverEntry(_In_ PDRIVER_OBJECT DriverObject, _In_ PUNICODE_STRING RegistryPath)` as entry point and do not decorated with `__declspec(safebuffers)`.
 No user functions decorated with `__declspec(safebuffers)`.
+
+## ARM64_CETShadowStack_NotApplicable.exe


add a empty line consistent with others

shaopeng-gh

michaelcfanning · 2022-06-12T18:32:47Z

src/Test.FunctionalTests.BinSkim.Rules/RuleTests.cs

-            this.VerifyApplicability(
+            HashSet<string> notApplicableArm64 = new HashSet<string>() { MetadataConditions.ImageIsArm64BitBinary };
+
+            this.VerifyApplicabililtyByConditionsOnly(


VerifyApplicabililtyByConditionsOnly

You are getting solidly into our test infrastructure if you understand this pattern. nice work. :) I'm sure Shaopeng may have been helpful in gaining 'the knowledge'.

michaelcfanning

Exclude ARM Binaries from BA2025 analysis

2412b46

marmegh requested review from eddynaka and michaelcfanning as code owners June 1, 2022 20:57

marmegh commented Jun 1, 2022

View reviewed changes

marmegh added 3 commits June 1, 2022 14:05

Cleanup ResultsTests.cs

bbc85b1

Update ReleaseHistory.md

f26ef21

Remove stray whitespace

bfd17a3

marmegh commented Jun 1, 2022

View reviewed changes

Cleanup validation method

07444d1

shaopeng-gh reviewed Jun 10, 2022

View reviewed changes

shaopeng-gh requested changes Jun 10, 2022

View reviewed changes

shaopeng-gh reviewed Jun 10, 2022

View reviewed changes

marmegh added 3 commits June 10, 2022 15:31

Added cross compile instructions for generating ARM and ARM64 test bi…

dd45441

…naries.

Removed blank line

075281f

Updated release history per PR feedback.

dae55f5

shaopeng-gh reviewed Jun 11, 2022

View reviewed changes

shaopeng-gh approved these changes Jun 11, 2022

View reviewed changes

michaelcfanning reviewed Jun 12, 2022

View reviewed changes

michaelcfanning approved these changes Jun 12, 2022

View reviewed changes

marmegh merged commit dcafdde into main Jun 13, 2022

marmegh deleted the users/marmegh/excludeArmBinariesForBA2025 branch June 13, 2022 19:15

marmegh mentioned this pull request Jun 21, 2022

Releasing v1.9.5 #655

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exclude ARM Binaries from BA2025 analysis #650

Exclude ARM Binaries from BA2025 analysis #650

marmegh commented Jun 1, 2022

marmegh Jun 1, 2022

michaelcfanning Jun 12, 2022

marmegh Jun 1, 2022

marmegh Jun 1, 2022

shaopeng-gh Jun 10, 2022 •

edited

Loading

shaopeng-gh Jun 10, 2022 •

edited

Loading

shaopeng-gh left a comment

shaopeng-gh Jun 10, 2022 •

edited

Loading

shaopeng-gh Jun 11, 2022

shaopeng-gh left a comment

michaelcfanning Jun 12, 2022

michaelcfanning left a comment

Exclude ARM Binaries from BA2025 analysis #650

Exclude ARM Binaries from BA2025 analysis #650

Conversation

marmegh commented Jun 1, 2022

marmegh Jun 1, 2022

Choose a reason for hiding this comment

michaelcfanning Jun 12, 2022

Choose a reason for hiding this comment

marmegh Jun 1, 2022

Choose a reason for hiding this comment

marmegh Jun 1, 2022

Choose a reason for hiding this comment

shaopeng-gh Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

shaopeng-gh Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

shaopeng-gh left a comment

Choose a reason for hiding this comment

shaopeng-gh Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

shaopeng-gh Jun 11, 2022

Choose a reason for hiding this comment

shaopeng-gh left a comment

Choose a reason for hiding this comment

michaelcfanning Jun 12, 2022

Choose a reason for hiding this comment

michaelcfanning left a comment

Choose a reason for hiding this comment

shaopeng-gh Jun 10, 2022 •

edited

Loading

shaopeng-gh Jun 10, 2022 •

edited

Loading

shaopeng-gh Jun 10, 2022 •

edited

Loading