From 2fddf88505cf95aaa1f83535d079ec9c5390948d Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 13 Oct 2023 11:00:37 -0700 Subject: [PATCH] Fix Sarif output of results with no rule match (#704) * Ensure sarif outputs results with no explicit rule matching * Adds option to use old behavior to not output results without explicit rule match Now should be consistent between json and sarif output. * Adds test case for new behavior --- Cli/AttackSurfaceAnalyzerClient.cs | 83 ++++++++++++++++++++++-------- Lib/Objects/CommandOptions.cs | 4 ++ Tests/ExportTests.cs | 22 ++++++-- 3 files changed, 83 insertions(+), 26 deletions(-) diff --git a/Cli/AttackSurfaceAnalyzerClient.cs b/Cli/AttackSurfaceAnalyzerClient.cs index e0575daf..f227a91d 100644 --- a/Cli/AttackSurfaceAnalyzerClient.cs +++ b/Cli/AttackSurfaceAnalyzerClient.cs @@ -715,6 +715,15 @@ private static ASA_ERROR RunExportCollectCommand(ExportCollectCommandOptions opt internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), ConcurrentBag> resultsIn, ExportOptions opts, string baseFileName, string analysesHash, IEnumerable rules) { var results = resultsIn.Select(x => new KeyValuePair($"{x.Key.Item1}_{x.Key.Item2}", x.Value)).ToDictionary(x => x.Key, x => x.Value); + if (opts.DisableImplicitFindings) + { + var resultKeys = resultsIn.Keys; + foreach (var key in resultKeys) + { + var newBag = new ConcurrentBag(resultsIn[key].Where(x => !x.Rules.Any())); + resultsIn[key] = newBag; + } + } JsonSerializer serializer = JsonSerializer.Create(new JsonSerializerSettings() { Formatting = Formatting.Indented, @@ -741,7 +750,7 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE string filePath = Path.Combine(path, AsaHelpers.MakeValidFileName(key)); if (opts.OutputSarif) { - WriteSarifLog(new Dictionary() { { key, results[key] } }, rules, filePath); + WriteSarifLog(new Dictionary() { { key, results[key] } }, rules, filePath, opts.DisableImplicitFindings); } else { @@ -762,12 +771,11 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE if (opts.OutputSarif) { string pathSarif = Path.Combine(outputPath, AsaHelpers.MakeValidFileName(baseFileName + "_summary.Sarif")); - WriteSarifLog(output, rules, pathSarif); + WriteSarifLog(output, rules, pathSarif, opts.DisableImplicitFindings); Log.Information(Strings.Get("OutputWrittenTo"), (new FileInfo(pathSarif)).FullName); } else { - using (StreamWriter sw = new(path)) //lgtm[cs/path-injection] { using JsonWriter writer = new JsonTextWriter(sw); @@ -785,9 +793,10 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE /// output of the analyzer result /// list of rules used /// file path of the Sarif log - public static void WriteSarifLog(Dictionary output, IEnumerable rules, string outputFilePath) + /// If the output should exclude results with no explicit level + internal static void WriteSarifLog(Dictionary output, IEnumerable rules, string outputFilePath, bool disableImplicitFindings) { - var log = GenerateSarifLog(output, rules); + var log = GenerateSarifLog(output, rules, disableImplicitFindings); var settings = new JsonSerializerSettings() { @@ -797,7 +806,7 @@ public static void WriteSarifLog(Dictionary output, IEnumerable< File.WriteAllText(outputFilePath, JsonConvert.SerializeObject(log, settings)); } - public static SarifLog GenerateSarifLog(Dictionary output, IEnumerable rules) + public static SarifLog GenerateSarifLog(Dictionary output, IEnumerable rules, bool disableImplicitFindings) { var metadata = (Dictionary)output["metadata"]; var results = (Dictionary)output["results"]; @@ -899,32 +908,62 @@ public static SarifLog GenerateSarifLog(Dictionary output, IEnum artifacts.Add(artifact); int index = artifacts.Count - 1; - foreach (var rule in compareResult.Rules) + if (compareResult.Rules.Any()) { - var sarifResult = new Result(); - sarifResult.Locations = new List() + foreach (var rule in compareResult.Rules) { - new Location() { - PhysicalLocation = new PhysicalLocation() - { - ArtifactLocation = new ArtifactLocation() + var sarifResult = new Result(); + sarifResult.Locations = new List() + { + new Location() { + PhysicalLocation = new PhysicalLocation() { - Index = index + ArtifactLocation = new ArtifactLocation() + { + Index = index + } } } - } - }; + }; - sarifResult.Level = GetSarifFailureLevel((ANALYSIS_RESULT_TYPE)rule.Severity); + sarifResult.Level = GetSarifFailureLevel((ANALYSIS_RESULT_TYPE)rule.Severity); - if (!string.IsNullOrWhiteSpace(rule.Name)) - { - sarifResult.RuleId = rule.Name; + if (!string.IsNullOrWhiteSpace(rule.Name)) + { + sarifResult.RuleId = rule.Name; + } + + sarifResult.Message = new Message() { Text = string.Format("{0}: {1} ({2})", rule.Name, compareResult.Identity, compareResult.ChangeType) }; + + sarifResults.Add(sarifResult); } + } + else + { + if (!disableImplicitFindings) + { + var sarifResult = new Result(); + sarifResult.Locations = new List() + { + new Location() { + PhysicalLocation = new PhysicalLocation() + { + ArtifactLocation = new ArtifactLocation() + { + Index = index + } + } + } + }; - sarifResult.Message = new Message() { Text = string.Format("{0}: {1} ({2})", rule.Name, compareResult.Identity, compareResult.ChangeType) }; + sarifResult.Level = GetSarifFailureLevel(compareResult.Analysis); + + sarifResult.RuleId = "Default Level"; + + sarifResult.Message = new Message() { Text = string.Format("Default Level: {0} ({1})", compareResult.Identity, compareResult.ChangeType) }; - sarifResults.Add(sarifResult); + sarifResults.Add(sarifResult); + } } } } diff --git a/Lib/Objects/CommandOptions.cs b/Lib/Objects/CommandOptions.cs index 27c4c367..b9df8eb5 100644 --- a/Lib/Objects/CommandOptions.cs +++ b/Lib/Objects/CommandOptions.cs @@ -258,6 +258,10 @@ public class ExportOptions : CommandOptions [Option(HelpText = "Output Sarif")] public bool OutputSarif { get; set; } + // Don't output results if they do not have an explicit rule applied + [Option(HelpText = "Ignore results with no explicit rule match")] + public bool DisableImplicitFindings { get; set; } + [Option(HelpText = "Force Analysis to be Single-Threaded")] public bool SingleThreadAnalysis { get; set; } diff --git a/Tests/ExportTests.cs b/Tests/ExportTests.cs index cba582b5..2a51e7e1 100644 --- a/Tests/ExportTests.cs +++ b/Tests/ExportTests.cs @@ -40,18 +40,32 @@ public void TestGenerateSarifLog() var outputDictionary = JsonConvert.DeserializeObject>(outputJson, jsonSettings); var rulesList = JsonConvert.DeserializeObject>(rulesJson, jsonSettings); - var sarif = AttackSurfaceAnalyzerClient.GenerateSarifLog(outputDictionary, rulesList); + var sarif = AttackSurfaceAnalyzerClient.GenerateSarifLog(outputDictionary, rulesList, true); - Assert.AreEqual(sarif.Runs[0].Artifacts.Count, 3); + Assert.AreEqual(3, sarif.Runs[0].Artifacts.Count); Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddText.txt")); Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddExe.exe")); Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestModifyExe.exe")); - Assert.AreEqual(sarif.Runs[0].Results.Count, 6); + Assert.AreEqual(6, sarif.Runs[0].Results.Count); Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing DEP: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)")); Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing ASLR: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)")); Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Unsigned binaries: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)")); Assert.IsFalse(sarif.Runs[0].Results.Any(r => r.Message.Text.Contains("TestAddText.txt"))); - Assert.AreEqual(sarif.Runs[0].Tool.Driver.Rules.Count, 41); + Assert.AreEqual(41, sarif.Runs[0].Tool.Driver.Rules.Count); + Assert.IsTrue(sarif.Runs[0].Tool.Driver.Rules.Any(r => r.FullDescription.Text == "Flag when privileged ports are opened.")); + + // Test with allowing impliciting findings + sarif = AttackSurfaceAnalyzerClient.GenerateSarifLog(outputDictionary, rulesList, false); + Assert.AreEqual(3, sarif.Runs[0].Artifacts.Count); + Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddText.txt")); + Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddExe.exe")); + Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestModifyExe.exe")); + Assert.AreEqual(7, sarif.Runs[0].Results.Count); + Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing DEP: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)")); + Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing ASLR: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)")); + Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Unsigned binaries: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)")); + Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text.Contains("TestAddText.txt"))); + Assert.AreEqual(41, sarif.Runs[0].Tool.Driver.Rules.Count); Assert.IsTrue(sarif.Runs[0].Tool.Driver.Rules.Any(r => r.FullDescription.Text == "Flag when privileged ports are opened.")); } }