Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Nessus + CIS CAT Converters #2574

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ejohn20
Copy link
Collaborator

@ejohn20 ejohn20 commented Nov 8, 2022

Creates a new converter + unit tests for Nessus (XML format) and CIS CAT (JSON format) covered in #2531

* feat: cis cat pro json converter

* bug: exclude passing tests from results for match forward baseline status

* feat: nessus converter v0

* bug: fingerprint targetid + empty plugin output

* feat: cis cat unit tests

* feat: nessus unit tests

* bug: omit 0 severity results + cvss override

* bug: cis cat set unknown status to warning

* feat: set rank for critical capability

* chore: update test cases

* chore: codeql cleanup

* chore: dotnet format errors

* feat: gh property tags
getResultSeverity(rule.Result, out level, out kind, out rank);

//Create only if a valid is assigned
if (rank != RankConstants.None)

Check warning

Code scanning / CodeQL

Equality check on floating point values

Equality checks on floating point values can yield unexpected results.
@ejohn20 ejohn20 changed the title 2531/converter nessus cis cat FEATURE: Nessus + CIS CAT Converters Nov 9, 2022
@yongyan-gh
Copy link
Collaborator

yongyan-gh commented Jan 6, 2023

@ejohn20 thanks for sharing the sample CisCat/Nessus SARIF files generated by the converters for review. Have couple suggestions pls review.

cc @michaelcfanning

@yongyan-gh
Copy link
Collaborator

yongyan-gh commented Jan 6, 2023

  1. Both converted CisCat and Nessus SARIF files' results don't contain location object in locations property.

GitHub Advanced Security code scanning will not display a result unless it provides a location that specifies the URI of the artifact that contains the result.

If plan to be ingested by GHAS pls consider generate a location for the results.

@yongyan-gh
Copy link
Collaborator

yongyan-gh commented Jan 6, 2023

  1. Considering adding a help URI for each rule through helpUri property of the rule object for both CisCat and Nessus SARIF log if possible.

Providing a URI where users can find detailed information about the rule helps users to understand the result and how they can best address it. The varies SARIF viewers can render a hyperlink for users easily navigate to the Uri contains detailed information.

E.g. the rule id RUSTSEC-2019-0001 has a help Uri 'https://rustsec.org/advisories/RUSTSEC-2019-0001'

@yongyan-gh
Copy link
Collaborator

  1. Please provide version information through 'version', 'semanticVersion', 'dottedQuadFileVersion' properties of driver object for Nessus SARIF log.

Providing version information enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions.

run.Tool.Driver.Rules = new List<ReportingDescriptor>();
foreach (CisCatRule rule in log.Rules)
{
run.Tool.Driver.Rules.Add(CreateReportDescriptor(rule));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run.Tool.Driver.Rules.Add(CreateReportDescriptor(rule));

Suggest to only log a rule if its referenced by a result and avoid logging duplicated rules.

This can be done in the results loop below and checking if the rule already exists in the `log.Rules' list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants