Skip to content
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

Vulnerabilities marked as fixed in distro packages should be reported as fixed for all contained packages too #1236

Closed
luhring opened this issue Apr 13, 2023 · 7 comments · Fixed by #1603
Labels
enhancement New feature or request

Comments

@luhring
Copy link
Contributor

luhring commented Apr 13, 2023

What happened:

Today Grype applies "fix" data from distro advisory sources to vulnerabilities matched to the distro package (e.g. an apk, rpm, etc.), which makes sense. But, it doesn't apply the fix data to the software that comprises the distro package. IMHO, this doesn't make sense.

Example

The Wolfi package py3.10-pip was matched to CVE-2018-20225, which is disputed. After looking into it, the Wolfi maintainers determined the correct action was to NAK the vulnerability in the Wolfi secdb.

When you run Grype against an image with this package, here's what happens:

$ grype -q cgr.dev/chainguard/python:3.10-dev
NAME         INSTALLED   FIXED-IN  TYPE    VULNERABILITY   SEVERITY
pip          23.0.1                python  CVE-2018-20225  High
python-3.10  3.10.11-r0            apk     CVE-2023-24329  High

Of note, all of the files that Grype lists as locations for pip (2 files in this case) are owned by the py3.10-pip Wolfi package, according to /lib/apk/db/installed.

What you expected to happen:

In this example, Grype shouldn't report the match of CVE-2018-20225 to the python pip package. It doesn't make sense for the distro package to be not affected by the vulnerability, but the python package described by the distro package to still be vulnerable to that same vulnerability.

How to reproduce it (as minimally and precisely as possible):

grype -q cgr.dev/chainguard/python:3.10-dev

Anything else we need to know?:

In case it helps for extra context / ringing any bells, I mentioned this to @spiffcs in Slack and to @wagoodman IRL recently. 😃

Environment:

  • Output of grype version:
Application:          grype
Version:              0.61.0
Syft Version:         v0.76.0
BuildDate:            2023-04-04T15:11:17Z
GitCommit:            d8c0c0805b59659c4d6e49d6806a0eba11bdc2ee
GitDescription:       v0.61.0
Platform:             darwin/arm64
GoVersion:            go1.19.7
Compiler:             gc
Supported DB Schema:  5
  • OS (e.g: cat /etc/os-release or similar): Windows Vista
@luhring luhring added the bug Something isn't working label Apr 13, 2023
@tgerla tgerla added this to OSS Apr 20, 2023
@spiffcs spiffcs added enhancement New feature or request and removed bug Something isn't working labels Jun 1, 2023
@spiffcs spiffcs moved this to Backlog in OSS Jun 1, 2023
@spiffcs
Copy link
Contributor

spiffcs commented Jun 1, 2023

Hey @luhring! I dug more into this again and you're exactly right that we should add this enhancement to grype. I think we're going to remove the bug label in favor of enhancement for the time being given that:

pip          23.0.1                python  CVE-2018-20225  High

is a correct match (version --> cpe constraint) in a vacuum.

The enhancement we want to make here is we turn off vulnerabilities in the specific instance where the locations of a vulnerable package are owned by some upstream package where fix data was applied.

I've added this to the backlog for the team as we come up with some solutions on the best way forward in applying this kind of logic when dealing with "fix" data from distro advisory sources.

@luhring
Copy link
Contributor Author

luhring commented Jun 2, 2023

That's awesome! Thanks, @spiffcs!

No problem with me on the issue label changes. The point that matters here is that it doesn't make sense for Grype to acknowledge the fix for the "distro package" metadata but not for other "ecosystem package" metadata, since both are just pointers to the same underlying code, and that code is either vulnerable or it's not. 😃

Let me know if I can help with this one. ❤️

@willmurphyscode
Copy link
Contributor

Hi @luhring and @spiffcs, this work is partially done: #1373 (implemented by #1387) removes packages from matching consideration if they are owned by an OS package, but it has a limitation that prevents it from fixing this yet: We can only remove packages if the distro being scanned has a feed that reports vulnerabilities that aren't fixed yet. (For example, Alpine and Amazon Linux feeds only report fixed vulnerabilities, so removing packages before matching there would result in false negatives.) So this fix is in, but not for Alpine.

In order to implement exactly what you want, grype needs to be changed to keep track of packages that it rejects as not matching a vulnerability and pass that state on to subsequent matchers. There's more discussion of that change here: #1426

TL;DR: The fix requested in this issue is in, but not for every distro, and the better fix will be unblocked by #1426.

@luhring
Copy link
Contributor Author

luhring commented Sep 11, 2023

Thanks for the update, @willmurphyscode!

The "negative match" concept sounds really cool. Just subscribed to that issue!

Just so I'm following, the reason to block this request on the "negative match" concept is because otherwise there'd be no hope for a grype explain kind of feature to express why a given vulnerability wasn't included in a Grype scan output in the case that the distro hadn't included the vulnerability in its feed?

@willmurphyscode
Copy link
Contributor

Just so I'm following, the reason to block this request on the "negative match" concept is because otherwise there'd be no hope for a grype explain kind of feature to express why a given vulnerability wasn't included in a Grype scan output in the case that the distro hadn't included the vulnerability in its feed?

@luhring I think that's right. Here's how I'm thinking of it: Right now, matchers do internal version filtering (example), but this filtering is discarded; the caller of the matcher can't tell if vulnerability wasn't a match for a package because of a search miss (grype DB has no data on this package for this matcher) or because a the version constraint excludes the match (grype DB knows the package was patched). So the implementation steps are:

  1. Update the signature on matchers so that they report packages they explicitly reject from matching ("negative matches")
  2. Add a later filter that does what you want: a negative match from the distro overrides a positive match from CPE or GHSA.

The reason for the big change is that, as matchers are currently implemented, when the distro matcher says a given package isn't vulnerable, the caller can't tell whether it's because the matcher knows about a fix, or because the matcher doesn't know about the vulnerability.

@luhring
Copy link
Contributor Author

luhring commented Sep 11, 2023

That makes sense. Looking forward to the new pattern, and let me know what I can do to help move this forward!

@mamccorm
Copy link

mamccorm commented Oct 4, 2023

+1 for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants