Burp parser aggregates findings on type

[fopinappb]

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

Line 42 in ca6628d

dupe_key = item.vuln_id_from_tool

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

Line 142 in ca6628d

vuln_id_from_tool = item_node.findall("type")[0].text

Burp parser aggregates all findings per type alone.
This means all Cross-site scripting (reflected) (for instance) fall under the same finding, regardless of endpoints, parameters or even severity.

I might be missing context on other people triage approach, but I find this confusing for mine as I'm unable to triage specific issues but rather an entire class of them.

Am I missing something?
Would a PR to change this be acceptable or too big of impact? Does it make sense to create a separate parser for this, if the latter? Or maybe make this dedupe_key configurable in settings.py?

[Maffooch]

What is your suggestion for updating the behavior? Are you thinking to aggregate by a combo of type and severity? I don't have much experience with the Burp parser, but the general intention of handling dynamic findings in dojo is to aggregate as many endpoints to a single finding as possible

[fopina]

I'll see if I can push a PR tomorrow as I've done the change locally anyway, easier to show.

but the general intention of handling dynamic findings in dojo is to aggregate as many endpoints to a single finding as possible

However, this statement is probably the root of my confusion: why is this so?

SAST findings are not aggregated by file path, why aggregate DAST ones per endpoint?

Using dojo for "discovery to resolution", I'd need to track each endpoint separately, as their resolutions might be different.
Even the teams involved, given a micro service architecture

Also, as the hash code fields do not consider endpoints for "uniqueness", nor description, it means that:

• 1 report with SQLi on endpoint A is submitted
• later, 1 report with SQLi on endpoints A and B is submitted

The second report is considered duplicate of first. But the first does not mention endpoint B, so it is missed.

And if it would include endpoints in the hash code fields, the opposite would happen:

• the second report would create a new finding that mentions an issue that already exists in dojo

Am I missing something in favor of aggregation?
Or is that approach motivated merely by finding/endpoint creation performance?

Once again, thanks for taking the time with such an out-of-nowhere "issue" 😀

[Maffooch]

However, this statement is probably the root of my confusion: why is this so?

The primary reason is value proposition. If a tool is generating 1000 results, and you import into Defectdojo, and get 100 aggregated findings, then DefectDojo is doing a lot of the organizing you would have to do otherwise

SAST findings are not aggregated by file path, why aggregate DAST ones per endpoint?

This is due to a historical architectural decision. Endpoints in DAST findings are stored in another database table, such that many endpoints can be associated with a single finding. File path and line number are single fields on a given finding, so there is really no possibility in aggregating those natively.

Also, as the hash code fields do not consider endpoints for "uniqueness", nor description, it means that:

• 1 report with SQLi on endpoint A is submitted
• later, 1 report with SQLi on endpoints A and B is submitted
  The second report is considered duplicate of first. But the first does not mention endpoint B, so it is missed.

And if it would include endpoints in the hash code fields, the opposite would happen:

• the second report would create a new finding that mentions an issue that already exists in dojo

This is where reimport will shine! However there are two use cases for controlling the behavior you (specifically) may or may not desire. The two cases are whether endpoints are used in the deduplication algorithm for a given tool.

• Endpoints are in the deduplication algorithm (the default - largely due to backwards compatibility)
  • On each reimport, the list of endpoints must match verbatim to be considered the same finding.
• Endpoints are not included in the dedupe algorithm (my preference as I exclusively use reimport)
  • Using your example of the SQLi finding with endpoint A, a reimport of the same finding with endpoint B will result in the original finding have both endpoint A and B. An third reimport that omits endpoint A would leave the same finding, but would mitigate endpoint A. This allows for a true point in time view of what endpoints are mitigated on a given finding

The nuance here is that are very valid use cases for each approach. I believe it comes down to using import vs reimport. My personal adage is that import is great for when you are getting audited, and reimport is great for when you are working with developers to mitigate issues. Reusing your SQLi example again:

• Import: Would likely create a lot of SQLi findings that would be difficult to manage, but I could know exactly what endpoints were vulnerable, and when they were vulnerable on a single finding without having to do much thinking
• Reimport: Little to no duplicates, and each finding operates as a "running document" of work to be done. If the finding is open, then at least one endpoint is vulnerable still. The potential downside is that an old finding could be reopened if new endpoints for a given vulnerability are rediscovered.

Each of those examples are wildly different use cases but serve as very valuable tools to have. The point I am trying to make is that DefectDojo is not really a one size fits all, but with the right tuning can be the best tool you have!

This may have been TMI, but I hope it's helpful!

[fopinappb]

Definitely not TMI but wanted info, appreciated the time taken.

These differences between import and reimport were the kind of differences I was looking for when I kicked off this thread in slack.
I think this should be documented (if it is I failed to find it) because it's a huge technical difference for handling imports, if import will lose details and reimport won't (for these cases which are not edge cases)

DefectDojo is doing a lot of the organizing you would have to do otherwise

My main issue with it is that it's not "organizing", it's just this parser behavior. Others will aggregate on other criteria (broader or narrower). Resulting in inconsistency of what a "finding" is expected to be (and hence its lifecycle).
To add to that is that this aggregation criteria is hardcoded in parser while HASH_CODE_FIELDS (for deduplication) is not, it can be configured via local_settings or even env variables, leading to deduplication+aggregation combinations that might be disfunctional. There might even be some parsers already set up in such way by default. I think I've seen a MR mentioning this issue too.

I would expect default behavior from parsers to be 1 dojo finding per 1 report finding (or as close as possible to it) and, then, I can create finding groups as I please. I could group by type as it is being done, but I could also group by endpoint or other field instead.

If they are already aggregated by the parser itself, I cannot un-aggregate them. If they are not aggregated by the parser, I can aggregate them with groups. To be flexible, I'd expect parsers to do less, not more, if that makes sense...

but with the right tuning can be the best tool you have

Problem here is that I don't see any tuning I can do for my case, where each endpoint has findings handled differently and by different developers/teams, apart from splitting the report myself before uploading into different engagements or tests...

Regardless I still agree it is the best tool I have for finding 😄