Skip to content

Commit

Permalink
🐛 fix semgrep severity logic DefectDojo#11218
Browse files Browse the repository at this point in the history
  • Loading branch information
@manuel-sommer
manuel-sommer committed Nov 7, 2024
1 parent 50d01bd commit 5c1450a
Showing 1 changed file with 0 additions and 5 deletions.
5 changes: 0 additions & 5 deletions dojo/tools/semgrep/parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,6 @@ def convert_severity(self, val):
if upper_value == "LOW":
return "Low"
if upper_value == "INFO":
if "WARNING" == val.upper():
return "Medium"
if "ERROR" == val.upper() or "HIGH" == val.upper():
return "High"
if "INFO" == val.upper():
return "Info"

Check failure on line 143 in dojo/tools/semgrep/parser.py

View workflow job for this annotation

GitHub Actions / ruff-linting

Ruff (E117) 

dojo/tools/semgrep/parser.py:143:1: E117 Over-indented
msg = f"Unknown value for severity: {val}"
raise ValueError(msg)
Expand Down

2 comments on commit 5c1450a

@bsterne
Copy link

@bsterne bsterne commented on 5c1450a Nov 8, 2024

Choose a reason for hiding this comment

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

I think this logic may still be incorrect:
https://semgrep.dev/docs/writing-rules/rule-syntax#required

One of the following values: INFO (Low severity), WARNING (Medium severity), or ERROR (High severity). The severity key specifies how critical are the issues that a rule potentially detects. Note: Semgrep Supply Chain differs, as its rules use CVE assignments for severity. For more information, see Filters section in Semgrep Supply Chain documentation.

So there may be some additional cases to consider, but I think the full logic is something closer to:

    if upper_value == "ERROR":
        return "High"
    if upper_value == "WARNING":
        return "Medium"
    if upper_value == "INFO":
        return "Low"

I'm willing to work on this with you if you want. I think we need to get some real sample reports and make sure we're covering all the possible cases. LMK if you want to collaborate on this. Thanks for picking this up!

@manuel-sommer
Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for the offer, I don't see any benefit in collaborating here. The PR is fine and got the first approvals.

Please sign in to comment.