Skip to content

Commit 2fddf88

Browse files
gfsgfs
authored
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
1 parent e922891 commit 2fddf88

File tree

3 files changed

+83
-26
lines changed

3 files changed

+83
-26
lines changed

‎Cli/AttackSurfaceAnalyzerClient.cs‎

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,15 @@ private static ASA_ERROR RunExportCollectCommand(ExportCollectCommandOptions opt
715715
internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), ConcurrentBag<CompareResult>> resultsIn, ExportOptions opts, string baseFileName, string analysesHash, IEnumerable<AsaRule> rules)
716716
{
717717
var results = resultsIn.Select(x => new KeyValuePair<string, object>($"{x.Key.Item1}_{x.Key.Item2}", x.Value)).ToDictionary(x => x.Key, x => x.Value);
718+
if (opts.DisableImplicitFindings)
719+
{
720+
var resultKeys = resultsIn.Keys;
721+
foreach (var key in resultKeys)
722+
{
723+
var newBag = new ConcurrentBag<CompareResult>(resultsIn[key].Where(x => !x.Rules.Any()));
724+
resultsIn[key] = newBag;
725+
}
726+
}
718727
JsonSerializer serializer = JsonSerializer.Create(new JsonSerializerSettings()
719728
{
720729
Formatting = Formatting.Indented,
@@ -741,7 +750,7 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE
741750
string filePath = Path.Combine(path, AsaHelpers.MakeValidFileName(key));
742751
if (opts.OutputSarif)
743752
{
744-
WriteSarifLog(new Dictionary<string, object>() { { key, results[key] } }, rules, filePath);
753+
WriteSarifLog(new Dictionary<string, object>() { { key, results[key] } }, rules, filePath, opts.DisableImplicitFindings);
745754
}
746755
else
747756
{
@@ -762,12 +771,11 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE
762771
if (opts.OutputSarif)
763772
{
764773
string pathSarif = Path.Combine(outputPath, AsaHelpers.MakeValidFileName(baseFileName + "_summary.Sarif"));
765-
WriteSarifLog(output, rules, pathSarif);
774+
WriteSarifLog(output, rules, pathSarif, opts.DisableImplicitFindings);
766775
Log.Information(Strings.Get("OutputWrittenTo"), (new FileInfo(pathSarif)).FullName);
767776
}
768777
else
769778
{
770-
771779
using (StreamWriter sw = new(path)) //lgtm[cs/path-injection]
772780
{
773781
using JsonWriter writer = new JsonTextWriter(sw);
@@ -785,9 +793,10 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE
785793
/// <param name="output">output of the analyzer result</param>
786794
/// <param name="rules">list of rules used</param>
787795
/// <param name="outputFilePath">file path of the Sarif log</param>
788-
public static void WriteSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules, string outputFilePath)
796+
/// <param name="disableImplicitFindings">If the output should exclude results with no explicit level</param>
797+
internal static void WriteSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules, string outputFilePath, bool disableImplicitFindings)
789798
{
790-
var log = GenerateSarifLog(output, rules);
799+
var log = GenerateSarifLog(output, rules, disableImplicitFindings);
791800

792801
var settings = new JsonSerializerSettings()
793802
{
@@ -797,7 +806,7 @@ public static void WriteSarifLog(Dictionary<string, object> output, IEnumerable<
797806
File.WriteAllText(outputFilePath, JsonConvert.SerializeObject(log, settings));
798807
}
799808

800-
public static SarifLog GenerateSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules)
809+
public static SarifLog GenerateSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules, bool disableImplicitFindings)
801810
{
802811
var metadata = (Dictionary<string, string>)output["metadata"];
803812
var results = (Dictionary<string, object>)output["results"];
@@ -899,32 +908,62 @@ public static SarifLog GenerateSarifLog(Dictionary<string, object> output, IEnum
899908

900909
artifacts.Add(artifact);
901910
int index = artifacts.Count - 1;
902-
foreach (var rule in compareResult.Rules)
911+
if (compareResult.Rules.Any())
903912
{
904-
var sarifResult = new Result();
905-
sarifResult.Locations = new List<Location>()
913+
foreach (var rule in compareResult.Rules)
906914
{
907-
new Location() {
908-
PhysicalLocation = new PhysicalLocation()
909-
{
910-
ArtifactLocation = new ArtifactLocation()
915+
var sarifResult = new Result();
916+
sarifResult.Locations = new List<Location>()
917+
{
918+
new Location() {
919+
PhysicalLocation = new PhysicalLocation()
911920
{
912-
Index = index
921+
ArtifactLocation = new ArtifactLocation()
922+
{
923+
Index = index
924+
}
913925
}
914926
}
915-
}
916-
};
927+
};
917928

918-
sarifResult.Level = GetSarifFailureLevel((ANALYSIS_RESULT_TYPE)rule.Severity);
929+
sarifResult.Level = GetSarifFailureLevel((ANALYSIS_RESULT_TYPE)rule.Severity);
919930

920-
if (!string.IsNullOrWhiteSpace(rule.Name))
921-
{
922-
sarifResult.RuleId = rule.Name;
931+
if (!string.IsNullOrWhiteSpace(rule.Name))
932+
{
933+
sarifResult.RuleId = rule.Name;
934+
}
935+
936+
sarifResult.Message = new Message() { Text = string.Format("{0}: {1} ({2})", rule.Name, compareResult.Identity, compareResult.ChangeType) };
937+
938+
sarifResults.Add(sarifResult);
923939
}
940+
}
941+
else
942+
{
943+
if (!disableImplicitFindings)
944+
{
945+
var sarifResult = new Result();
946+
sarifResult.Locations = new List<Location>()
947+
{
948+
new Location() {
949+
PhysicalLocation = new PhysicalLocation()
950+
{
951+
ArtifactLocation = new ArtifactLocation()
952+
{
953+
Index = index
954+
}
955+
}
956+
}
957+
};
924958

925-
sarifResult.Message = new Message() { Text = string.Format("{0}: {1} ({2})", rule.Name, compareResult.Identity, compareResult.ChangeType) };
959+
sarifResult.Level = GetSarifFailureLevel(compareResult.Analysis);
960+
961+
sarifResult.RuleId = "Default Level";
962+
963+
sarifResult.Message = new Message() { Text = string.Format("Default Level: {0} ({1})", compareResult.Identity, compareResult.ChangeType) };
926964

927-
sarifResults.Add(sarifResult);
965+
sarifResults.Add(sarifResult);
966+
}
928967
}
929968
}
930969
}

‎Lib/Objects/CommandOptions.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,10 @@ public class ExportOptions : CommandOptions
258258
[Option(HelpText = "Output Sarif")]
259259
public bool OutputSarif { get; set; }
260260

261+
// Don't output results if they do not have an explicit rule applied
262+
[Option(HelpText = "Ignore results with no explicit rule match")]
263+
public bool DisableImplicitFindings { get; set; }
264+
261265
[Option(HelpText = "Force Analysis to be Single-Threaded")]
262266
public bool SingleThreadAnalysis { get; set; }
263267

‎Tests/ExportTests.cs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,32 @@ public void TestGenerateSarifLog()
4040
var outputDictionary = JsonConvert.DeserializeObject<Dictionary<string, object>>(outputJson, jsonSettings);
4141
var rulesList = JsonConvert.DeserializeObject<IEnumerable<AsaRule>>(rulesJson, jsonSettings);
4242

43-
var sarif = AttackSurfaceAnalyzerClient.GenerateSarifLog(outputDictionary, rulesList);
43+
var sarif = AttackSurfaceAnalyzerClient.GenerateSarifLog(outputDictionary, rulesList, true);
4444

45-
Assert.AreEqual(sarif.Runs[0].Artifacts.Count, 3);
45+
Assert.AreEqual(3, sarif.Runs[0].Artifacts.Count);
4646
Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddText.txt"));
4747
Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddExe.exe"));
4848
Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestModifyExe.exe"));
49-
Assert.AreEqual(sarif.Runs[0].Results.Count, 6);
49+
Assert.AreEqual(6, sarif.Runs[0].Results.Count);
5050
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing DEP: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)"));
5151
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing ASLR: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)"));
5252
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Unsigned binaries: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)"));
5353
Assert.IsFalse(sarif.Runs[0].Results.Any(r => r.Message.Text.Contains("TestAddText.txt")));
54-
Assert.AreEqual(sarif.Runs[0].Tool.Driver.Rules.Count, 41);
54+
Assert.AreEqual(41, sarif.Runs[0].Tool.Driver.Rules.Count);
55+
Assert.IsTrue(sarif.Runs[0].Tool.Driver.Rules.Any(r => r.FullDescription.Text == "Flag when privileged ports are opened."));
56+
57+
// Test with allowing impliciting findings
58+
sarif = AttackSurfaceAnalyzerClient.GenerateSarifLog(outputDictionary, rulesList, false);
59+
Assert.AreEqual(3, sarif.Runs[0].Artifacts.Count);
60+
Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddText.txt"));
61+
Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestAddExe.exe"));
62+
Assert.IsTrue(sarif.Runs[0].Artifacts.Any(a => a.Location.Description.Text == "C:\\Test\\Scan2\\TestModifyExe.exe"));
63+
Assert.AreEqual(7, sarif.Runs[0].Results.Count);
64+
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing DEP: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)"));
65+
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Missing ASLR: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)"));
66+
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text == "Unsigned binaries: C:\\Test\\Scan2\\TestAddExe.exe (CREATED)"));
67+
Assert.IsTrue(sarif.Runs[0].Results.Any(r => r.Message.Text.Contains("TestAddText.txt")));
68+
Assert.AreEqual(41, sarif.Runs[0].Tool.Driver.Rules.Count);
5569
Assert.IsTrue(sarif.Runs[0].Tool.Driver.Rules.Any(r => r.FullDescription.Text == "Flag when privileged ports are opened."));
5670
}
5771
}

0 commit comments

Comments
 (0)