-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(vex): use a separate visited set for each DFS path
#9760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(vex): use a separate visited set for each DFS path
#9760
Conversation
visited set for each tree path.
visited set for each tree path.visited set for each DFS path
| name: "check one parent from multiple dependency paths", | ||
| args: args{ | ||
| // - oci:debian?tag=12 | ||
| // - pkg:deb/debian/[email protected] | ||
| // - pkg:deb/debian/[email protected] | ||
| // - pkg:deb/debian/[email protected] | ||
| // - pkg:deb/debian/[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case looks like multiple parents ([email protected] and [email protected]) from a single dependency ([email protected]). Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right.
But oci:debian?tag=12 is the parent for both [email protected] and [email protected].
For this test case there are two dependency paths:
- pkg:deb/debian/[email protected] -> pkg:deb/debian/[email protected] -> oci:debian?tag=12
- pkg:deb/debian/[email protected] -> pkg:deb/debian/[email protected] -> oci:debian?tag=12
After the first path, we mark oci:debian?tag=12 as visited.
When we process pkg:deb/debian/[email protected], we see that oci:debian?tag=12 is already visited and return true.
I reviewed the change and I’m leaning toward this idea. However, it's not good to often change the SBOM structure. What do you think? |
|
I also don’t like changing the logic back and forth (since #9007 is basically a revert of the changes as well). However, we still need to fix #9011 somehow. We can consider the first fix option (9f4702c).
|
Description
This PR fixes a critical bug in VEX (Vulnerability Exploitability eXchange) processing where the visited set was being shared across different dependency paths during depth-first search traversal. The issue caused incorrect
vulnerability filtering when multiple parents had dependencies on the same component.
Reasons
The original implementation had a fundamental flaw where the visited map was shared across all recursive paths in the DFS traversal. This meant that if component A was visited through one parent path, it would be marked as visited for
all other parent paths, potentially causing the algorithm to incorrectly conclude that a component couldn't reach the root when it actually could through a different path.
This bug was particularly problematic in scenarios with diamond dependencies (multiple paths to the same component) and could lead to incorrect vulnerability filtering decisions in VEX processing.
Related issues
Related PRs
Checklist