Skip to content

Commit

Permalink
Fix Sarif output of results with no rule match (#704)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@gfs
gfs committed Oct 13, 2023
1 parent e922891 commit 2fddf88
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 26 deletions.
83 changes: 61 additions & 22 deletions Cli/AttackSurfaceAnalyzerClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,15 @@ private static ASA_ERROR RunExportCollectCommand(ExportCollectCommandOptions opt
internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), ConcurrentBag<CompareResult>> resultsIn, ExportOptions opts, string baseFileName, string analysesHash, IEnumerable<AsaRule> rules)
{
var results = resultsIn.Select(x => new KeyValuePair<string, object>($"{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<CompareResult>(resultsIn[key].Where(x => !x.Rules.Any()));
resultsIn[key] = newBag;
}
}
JsonSerializer serializer = JsonSerializer.Create(new JsonSerializerSettings()
{
Formatting = Formatting.Indented,
Expand All @@ -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<string, object>() { { key, results[key] } }, rules, filePath);
WriteSarifLog(new Dictionary<string, object>() { { key, results[key] } }, rules, filePath, opts.DisableImplicitFindings);
}
else
{
Expand All @@ -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);
Expand All @@ -785,9 +793,10 @@ internal static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE
/// <param name="output">output of the analyzer result</param>
/// <param name="rules">list of rules used</param>
/// <param name="outputFilePath">file path of the Sarif log</param>
public static void WriteSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules, string outputFilePath)
/// <param name="disableImplicitFindings">If the output should exclude results with no explicit level</param>
internal static void WriteSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules, string outputFilePath, bool disableImplicitFindings)
{
var log = GenerateSarifLog(output, rules);
var log = GenerateSarifLog(output, rules, disableImplicitFindings);

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

public static SarifLog GenerateSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules)
public static SarifLog GenerateSarifLog(Dictionary<string, object> output, IEnumerable<AsaRule> rules, bool disableImplicitFindings)
{
var metadata = (Dictionary<string, string>)output["metadata"];
var results = (Dictionary<string, object>)output["results"];
Expand Down Expand Up @@ -899,32 +908,62 @@ public static SarifLog GenerateSarifLog(Dictionary<string, object> 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<Location>()
foreach (var rule in compareResult.Rules)
{
new Location() {
PhysicalLocation = new PhysicalLocation()
{
ArtifactLocation = new ArtifactLocation()
var sarifResult = new Result();
sarifResult.Locations = new List<Location>()
{
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<Location>()
{
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);
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions Lib/Objects/CommandOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
22 changes: 18 additions & 4 deletions Tests/ExportTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,32 @@ public void TestGenerateSarifLog()
var outputDictionary = JsonConvert.DeserializeObject<Dictionary<string, object>>(outputJson, jsonSettings);
var rulesList = JsonConvert.DeserializeObject<IEnumerable<AsaRule>>(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."));
}
}
Expand Down

0 comments on commit 2fddf88

Please sign in to comment.