microsoft / DevSkim

DevSkim is a set of IDE plugins, language analyzers, and rules that provide security "linting" capabilities.
MIT License
910 stars 116 forks source link

Devskim is only reporting errors with no severity #605

Closed Sof0-0 closed 8 months ago

Sof0-0 commented 8 months ago

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!

gfs commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

@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.