Issue #2794: fix different result for affected projects of aliases

[sephiroth-j]

Description

The list of projects affected by a specific vulnerability now also contains projects that are affected by aliases of the specified vulnerability. This ensures that the list of affected projects is the same for all aliases of a vulnerability.

Addressed Issue

fixes #2794

Additional Details

Checklist

• I have read and understand the contributing guidelines
• This PR fixes a defect, and I have provided tests to verify that the fix is effective
• This PR implements an enhancement, and I have provided tests to verify that it works as intended
• This PR introduces changes to the database model, and I have added corresponding update logic
• This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.03% (target: -1.00%)	✅ 92.86% (target: 70.00%)

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (72e582c)	23013	18283	79.45%
Head commit (02787d4)	23016 (+3)	18292 (+9)	79.48% (+0.03%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#4517)	28	26	92.86%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

[sephiroth-j]

Hi @nscuro, is there any chance of releasing this as part of version 4.12.3? It would be great to have this along with DependencyTrack/frontend/issues/481 in the same release.

[nscuro]

@sephiroth-j My concern with this is that iterating over vuln.getComponents() is already super expensive and can cause timeouts in larger portfolios. This will only be amplified by repeating the same operation for all aliases.

We recently reworked this query in Hyades to make it more efficient. It works really well for huge portfolios, but it does lean on some Postgres-specific SQL syntax.

I'm not sure how realistic it is to port this query over, but we will need to do something here so the cost doesn't explode in our faces. If we can make the existing query logic in v4 more efficient, we would have a bit more headroom to add the change you're proposing.

[sephiroth-j]

Would it help to add an option to DT or a query parameter that controls whether aliases are considered or not? It would be easy to adjust the mapMulti operation accordingly. However, this would also require changes to the UI.

[valentijnscholten]

Another thing to take into account is that the approach may work well for when the list of affected projects for all aliases is the same. If the list is different, for example due to a difference in affected versions, it might actually be confusing or limiting users to triage the vulnerabilities. Would it make sense to have some kind of flag or two separate lists to tell the different between a "direct" affection and a "alias" affection? If we feel combining them in one call would be too expensive, it could be two calls? Maybe the alias affected list behind a new tab / button?

It still feels kinda weird to having to this because the datamodel is not really taking performance of these kinda of queries into account, but it's not something that can be easily changed.

[sephiroth-j]

Hello @valentijnscholten ,

If the list is different, for example due to a difference in affected versions, ....

But that is precisely the point. The result should include every project, especially those where the alias is the primary associated vulnerability. If vulnerability v' is an alias of vulnerability v and vice versa, then the result of affected projects of v and v' must be the same (assuming the user has access to all of them). Anything else is not plausible and contradicts the fact that both vulnerabilities are aliases of each other.

Would it make sense to have some kind of flag or two separate lists to tell the different between a "direct" affection and a "alias" affection?

I was thinking of a flag (checkbox) that is disabled for now and can be enabled by the user to include projects of aliases. Later, when the performance issue is resolved, it can be removed and/or enabled by default. This would be a minimal change to the frontend.

If we feel combining them in one call would be too expensive, it could be two calls?

That would also be possible. But then both lists should be merged into one - which can only be done via the frontend when both requests are completed and until then no results can be displayed. The UX of two separate lists on separate tabs would be bad because it makes it difficult to compare the lists and puts the burden on the user to combine them.

[valentijnscholten]

In a perfect world the aliases are indeed identical. But if that is always the case, the list of affected projects is already identical and doesn't need any merging?

I do think it's good to also cover the case where the list of affected projects is different, but I think we should try to not lose the information about which alias was applied and which wasn't. It can also be interesting for DT developers or the security team managing DT to see why a certain CVE has 20 affected projects but the GHSA alias has 30 affected projects.

Without having dived into the details, even with the current datamodel I think it should be possible to find affected components for a list of Vulnerability IDs instead of a single ID without being orders of magnitude slower.

[valentijnscholten]

If the list of affected projects is different across aliases, probably the list of affected components is also different. Should those be merged as well?