Skip to content
New issue

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

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

Already on GitHub? # to your account

Devskim is only reporting errors with no severity #605

Closed
Sof0-0 opened this issue Feb 23, 2024 · 6 comments · Fixed by #606
Closed

Devskim is only reporting errors with no severity #605

Sof0-0 opened this issue Feb 23, 2024 · 6 comments · Fixed by #606
Labels

Comments

@Sof0-0
Copy link

Sof0-0 commented Feb 23, 2024

Devskim returned more than 300+ alerts and some of them are critical with weak hash type. Is it possible to display the CWE and severity level for it? Here is the configuration file I am using:
Screenshot 2024-02-22 at 21 35 56

Thank you!

@Sof0-0 Sof0-0 added the bug label Feb 23, 2024
@gfs
Copy link
Contributor

gfs commented Feb 23, 2024

Sorry you're not seeing what you'd expect. Can you clarify which rule I particular you're getting findings for - and if you believe those findings are false positives? Or are you looking for more information on the rationale for the provided findings?

I did a quick check locally and the level value of the sarif result object (which corresponds to the severity in sarif) is being set as far as I can tell. Though "critical" is not one of the values available for the level field - I believe devskim maps a 'critical' devskim severity to sarif 'error' level.

@Sof0-0
Copy link
Author

Sof0-0 commented Feb 23, 2024

Absolutely! Here are some rules with no severity displayed:

Rule ID
DS111237

Rule ID
DS126858

Rule ID
DS173237

Rule ID
DS440010

And so on.. None of them display severities (just an Error):
Screenshot 2024-02-23 at 09 57 09

@scovetta
Copy link
Member

From https://docs.github.com/en/code-security/code-scanning/managing-code-scanning-alerts/about-code-scanning-alerts, it looks like findings can be either error/warning/note, but CodeQL findings can be critical/high/medium/low. I'm not sure if this is a hard restriction, worst case we might be able to add an indicator to the rule title -- e.g. "[high] A weak or broken hash algorithm was detected".

@gfs
Copy link
Contributor

gfs commented Feb 23, 2024

@Sof0-0 Thanks for the screenshot that helps clarify what you're seeing. We currently only populate the Level of the finding which as Mike notes can only be Error, Warning or Note, so it is currently worked as expected, in this case Error is the severity level of the result. But I appreciate the feedback that you'd like more contextual information.

It looks there is room for improvement here though, we could also populate the properties.precision, properties.problem.severity and properties.security-severity inside the reported rule descriptors. https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#reportingdescriptor-object.

Precision is a easy mapping from the existing confidence field in DevSkim rules, and properties.problem.severity is also a easy mapping from the existing devskim rule severity field. The security-severity field is a bit more difficult, because the range is a float - this isn't a field we have a clear parallel to in existing devskim rules, so it may require more thought about how to populate that. I'll work on an update that populates the first two initially.

@Sof0-0
Copy link
Author

Sof0-0 commented Feb 23, 2024

Thank you so much for this response! Can I ask what would be the approximate timeline of such fix so I can plan the solution accordingly?

@gfs
Copy link
Contributor

gfs commented Feb 23, 2024

@Sof0-0 I expect I can release an update next week that will populate properties.problem.severity, and possibly confidence. I dug into confidence a bit more and it may take a bit more work than anticipated, as we've tended to define confidence at the pattern level (which means its more appropriate to assign it to specific findings, but that's not available as an option for the sarif where it must be defined at the rule level), but I'll try to find a good solution for that as well.

I don't an estimate at this time about when we could populate security-severity.

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

Successfully merging a pull request may close this issue.

3 participants