semgrep: hash code fields

[fopinappb]

django-DefectDojo/dojo/tools/semgrep/parser.py

Line 52 in 2f25c45

NOTE: uses legacy dedupe: ['title', 'cwe', 'line', 'file_path', 'description']

I see semgrep parser is using legacy hash code fields instead of custom list.

I know I can change this in my local settings (setting HASHCODE_FIELDS_PER_SCANNER for Semgrep JSON Report) but I'd like to understand if there's reasoning not to do it (or if it was just missed / noone felt impacted).

I believe CWE makes no sense for semgrep because each rule always has the same CWE.

Also, I think having line make de-duplication fail very often (as anything added before the code snippet would change it and create new finding).

[valentijnscholten]

Semgrep should use DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL_OR_HASH_CODE

django-DefectDojo/dojo/settings/settings.dist.py

Line 1716 in 4decd88

"Semgrep JSON Report": DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL_OR_HASH_CODE,

[fopina]

Right I posted that to list the fields but not the right place

django-DefectDojo/dojo/settings/settings.dist.py

Line 1394 in 4decd88

# If not present, default is the legacy behavior: see models.py, compute_hash_code_legacy function

There are no fields defined for semgrep, so hash code legacy uses those (same as in parser, but I guess coincidence)

Isn’t that correct?
So my question would still apply

[valentijnscholten]

Good point. I think it's just because nobody ever looked in detail at it and/or the default ("legacy") set of fields work ok. Removing cwe could be ok but won't bring much benefit if it's the same value always. Removing the line number is the classic trade-off for SAST reports where removing it would possibly collapse multiple different findings into one resulting in duplicates. Keeping it could lead to findings being opened/closed when the line number changes. I wonder if it could work to treat line numbers as endpoints/locations (as a bigger future change).