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

[Feat]: Expose PkgPath value in CVE results #2175

Closed
andaaron opened this issue Jan 22, 2024 · 21 comments
Closed

[Feat]: Expose PkgPath value in CVE results #2175

andaaron opened this issue Jan 22, 2024 · 21 comments
Assignees
Labels
feature New feature or request
Milestone

Comments

@andaaron
Copy link
Contributor

Is your feature request related to a problem? Please describe.

https://github.com/aquasecurity/trivy/blob/a96f66f176e512ffb029f2d421e2d77b805eb6ee/pkg/types/vulnerability.go#L14

Show it along the package name in the ZLI and ZUI results.

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

@andaaron andaaron added the feature New feature or request label Jan 22, 2024
@vrajashkr
Copy link
Contributor

Hi @andaaron, @rchincha, would it be alright if I had a go at working on this issue?

@andaaron
Copy link
Contributor Author

andaaron commented Feb 9, 2024

Hi @vrajashkr, please go ahead, thank you.

@andaaron
Copy link
Contributor Author

andaaron commented Feb 9, 2024

Please let us know if you have any questions.

On server side you need to:

Right now the cli side doesn't show package information for CVEs, but maybe we should start doing that.
On client side you would need to:

This last part needs a discussion about how the output should look like, as right now we show a CVE per line, but a single CVE may be found in multiple packages.

@vrajashkr
Copy link
Contributor

Thanks for the detailed information @andaaron ! It's very helpful.

I'll get back with any questions as I work through the changes.

@vrajashkr
Copy link
Contributor

vrajashkr commented Feb 10, 2024

I've made a set of changes. The code successfully compiles and the cve command seems to execute without panicking. Yay!

I'll raise a PR with my changes for some early feedback after writing a few Unit tests.

We can probably discuss how we'd like to show it in the output once we see some examples with actual images.

@vrajashkr
Copy link
Contributor

vrajashkr commented Feb 11, 2024

While testing, these were some of the package paths I saw for the test image I used (https://hub.docker.com/r/fl73/spring4shell-vulnerable-app):

usr/local/tomcat/webapps/spring4shell.war/WEB-INF/lib-provided/tomcat-embed-core-9.0.56.jar
usr/local/tomcat/webapps/spring4shell.war/WEB-INF/lib/spring-boot-2.6.3.jar
usr/local/tomcat/lib/catalina.jar
usr/local/tomcat/webapps/spring4shell.war/WEB-INF/lib/snakeyaml-1.29.jar
usr/local/tomcat/webapps/spring4shell.war/WEB-INF/lib/spring-expression-5.3.15.jar

One thought I had was to use the listing CLI to show a count of the vulnerable packages and have a new CLI to show more details (including the package paths and versions).
For example, something like:
Existing CLI enhanced to show count of vulnerable packages:

bin/zli-linux-amd64 cve --config main list fl73/spring4shell-vulnerable-app:latest
CRITICAL 28, HIGH 69, MEDIUM 66, LOW 83, UNKNOWN 1, TOTAL 247

ID                SEVERITY  TITLE                                             VULNERABLE PACKAGE COUNT
CVE-2016-1000027  CRITICAL  spring: HttpInvokerServiceExporter readRemote...  1
CVE-2019-8457     CRITICAL  heap out-of-bound read in function rtreenode()    1
CVE-2021-22945    CRITICAL  curl: use-after-free and double-free in MQTT ...  3

New CLI that accepts a CVE as an argument for more details

./bin/zli-linux-amd64 cve --config main detail fl73/spring4shell-vulnerable-app:latest CVE-2016-1000027

This could output:

CVE ID: CVE-2016-1000027
Severity: CRITICAL
Title: <CVE TITLE>
Description: <CVE DESCRIPTION>

Vulnerable Packages:
NAME                PATH                       INSTALLED VERSION   FIXED VERSION
Package1            /usr/bin/example.jar       1.0.0               2.0.0

Would love to get your thoughts on this @andaaron.

By the way, let me know if we would like to take up the "Display CVE package information in ZLI" as a separate issue and PR first before we add the package path to it.

@andaaron
Copy link
Contributor Author

andaaron commented Feb 11, 2024

From my point of view it could be a separate PR.

Either your suggestion of having a separate command, of we do something similar to what we did for showing layer digests in image command output (have a verbose command line parameter to show extra information).

ID                SEVERITY  TITLE                                             VULNERABLE PACKAGE  INSTALLED VERSION   FIXED VERSION  PATH
CVE-2016-1000027  CRITICAL  spring: HttpInvokerServiceExporter readRemote... 
                                                                              Package1            1.0.0               2.0.0 /usr/bin/example.jar
                                                                              Package2            1.0.0               2.0.0 /usr/bin/example2.jar                                                                           
CVE-2019-8457     CRITICAL  heap out-of-bound read in function rtreenode()
                                                                              Package3            1.0.1               1.0.2 /usr/bin/example2.jar       
CVE-2021-22945    CRITICAL  curl: use-after-free and double-free in MQTT ...
                                                                              Package4            3.0.0               23.0.1 /usr/bin/example2.jar      

@rchincha, what do you think?

@vrajashkr
Copy link
Contributor

vrajashkr commented Feb 16, 2024

#2241 brings in backend support for fetching PkgPath information from the scan report.

@andaaron, since the ZLI approach to display this in CLI output is still in discussion, shall I take a look at the ZUI changes to display this information? I see that ZUI already supports listing the packages for a vulnerability.

@andaaron
Copy link
Contributor Author

Yes, I think we can add an extra column in the packages table.
My only concern is how that will fit in on smaller screens, but let's give it a shot.

@vrajashkr
Copy link
Contributor

I had a go at adding Package Path to the existing table and there were some challenges. It doesn't seem to be fitting too well even on larger screens when the vulnerability package name and/or the path are long.

Here's a sample with a short path (our spring-web image):

On Desktop

image

On Mobile

image

Here's a sample with a longer path (fl73/spring4shell-vulnerable-app):

On Desktop

image

Seeing that we have quite a bit of data to present now, I tried out making some changes to the display to make it vertical instead of horizontally stacked.

Here's what it looks like after some trial changes:

On Desktop

image

On Mobile

image

I'm open to feedback and suggestions on how we could present this information.

@rchincha
Copy link
Contributor

From my point of view it could be a separate PR.

Either your suggestion of having a separate command, of we do something similar to what we did for showing layer digests in image command output (have a verbose command line parameter to show extra information).

ID                SEVERITY  TITLE                                             VULNERABLE PACKAGE  INSTALLED VERSION   FIXED VERSION  PATH
CVE-2016-1000027  CRITICAL  spring: HttpInvokerServiceExporter readRemote... 
                                                                              Package1            1.0.0               2.0.0 /usr/bin/example.jar
                                                                              Package2            1.0.0               2.0.0 /usr/bin/example2.jar                                                                           
CVE-2019-8457     CRITICAL  heap out-of-bound read in function rtreenode()
                                                                              Package3            1.0.1               1.0.2 /usr/bin/example2.jar       
CVE-2021-22945    CRITICAL  curl: use-after-free and double-free in MQTT ...
                                                                              Package4            3.0.0               23.0.1 /usr/bin/example2.jar      

@rchincha, what do you think?

This information is actually quite useful. Comments as follows:

  1. Move the PATH column right after the PACKAGE column
  2. INSTALL-VER and FIXED-VER - shorter columns and single words and move these to the end
  3. If FIXED-VER is empty, then add a "-" for visual feedback

@rchincha
Copy link
Contributor

@vrajashkr The final UI changes look better and lgtm.

@rchincha
Copy link
Contributor

@vrajashkr let us know when you have PRs ready for these.

@vrajashkr
Copy link
Contributor

Thanks for the comments @rchincha. Will raise PRs once the changes are ready.

@vrajashkr
Copy link
Contributor

vrajashkr commented Mar 1, 2024

Hi @andaaron, @rchincha, I tried out the ZLI changes for the package details. One thing I observed is that vulnerable paths in an image may be quite long so the user may not get much information from the listing as they don't see the full path to the artifact.
This is the case for the "fixed version" as well - it gets truncated by the ellipses when they get too long.

This approach worked fine for the digest layers example as layer hashes are still valid as references when shortened.

On the flip side, if the allowed width is set too high, the screen becomes challenging to work with as we may see wrapping/truncation depending on the terminal config.

Here's an example screenshot:
image

Do you happen to have any thoughts on how we could approach the output width for this?

@andaaron
Copy link
Contributor Author

andaaron commented Mar 1, 2024

Maybe we should try your initial suggestion in #2175 (comment)

@vrajashkr
Copy link
Contributor

vrajashkr commented Mar 2, 2024

I gave it a go, but I did make one change. In the above comment, I had earlier suggested the use of a table for the package list, but as that faces the same trouble of knowing the right length to truncate, I decided to do something similar to what was done in zui - a vertical presentation. I'd love to get your thoughts on it.

Here's a sample:

bin/zli-linux-amd64 --config main cve list example-image --verbose
CRITICAL 28, HIGH 75, MEDIUM 66, LOW 84, UNKNOWN 1, TOTAL 254

CVE-2016-1000027
Severity: CRITICAL
Title: spring: HttpInvokerServiceExporter readRemoteInvocation method untrusted java deserialization
Description:
Pivotal Spring Framework through 5.3.16 suffers from a potential remote code execution (RCE) issue if used for Java deserialization of untrusted data. Depending on how the library is implemented within a product, this issue may or not occur, and authentication may be required. NOTE: the vendor's position is that untrusted data is not an intended use case. The product's behavior will not be changed because some users rely on deserialization of trusted data.

Vulnerable Packages:
 Package Name: org.springframework:spring-web
 Package Path: usr/local/tomcat/webapps/spring4shell.war/WEB-INF/lib/spring-web-5.3.15.jar
 Installed Version: 5.3.15
 Fixed Version: 6.0.0


CVE-2019-8457
Severity: CRITICAL
Title: heap out-of-bound read in function rtreenode()
Description:
SQLite3 from 3.6.0 to and including 3.27.2 is vulnerable to heap out-of-bound read in the rtreenode() function when handling invalid rtree tables.

Vulnerable Packages:
 Package Name: libdb5.3
 Package Path: Not Specified
 Installed Version: 5.3.28+dfsg1-0.8
 Fixed Version: Not Specified


CVE-2021-22945
Severity: CRITICAL
Title: curl: use-after-free and double-free in MQTT sending
Description:
When sending data to an MQTT server, libcurl <= 7.73.0 and 7.78.0 could in some circumstances erroneously keep a pointer to an already freed memory area and both use that again in a subsequent call to send data and also free it *again*.

Vulnerable Packages:
 Package Name: curl
 Package Path: Not Specified
 Installed Version: 7.74.0-1.3+deb11u1
 Fixed Version: 7.74.0-1.3+deb11u2

 Package Name: libcurl3-gnutls
 Package Path: Not Specified
 Installed Version: 7.74.0-1.3+deb11u1
 Fixed Version: 7.74.0-1.3+deb11u2

 Package Name: libcurl4
 Package Path: Not Specified
 Installed Version: 7.74.0-1.3+deb11u1
 Fixed Version: 7.74.0-1.3+deb11u2


CVE-2021-29921
Severity: CRITICAL
Title: python-ipaddress: Improper input validation of octal strings
Description:
In Python before 3,9,5, the ipaddress library mishandles leading zero characters in the octets of an IP address string. This (in some situations) allows attackers to bypass access control that is based on IP addresses.

Vulnerable Packages:
 Package Name: libpython3.9-minimal
 Package Path: Not Specified
 Installed Version: 3.9.2-1
 Fixed Version: Not Specified

 Package Name: libpython3.9-stdlib
 Package Path: Not Specified
 Installed Version: 3.9.2-1
 Fixed Version: Not Specified

 Package Name: python3.9
 Package Path: Not Specified
 Installed Version: 3.9.2-1
 Fixed Version: Not Specified

 Package Name: python3.9-minimal
 Package Path: Not Specified
 Installed Version: 3.9.2-1
 Fixed Version: Not Specified

@vrajashkr
Copy link
Contributor

Hi @andaaron, @rchincha, gentle bump on this. Any thoughts on the shared output? Thanks!

@rchincha
Copy link
Contributor

rchincha commented Mar 5, 2024

@vrajashkr thanks for all the work you are doing.

I would suggest keeping both the summary/table view and the --verbose view.
The summary/table view can have the fixed-width plus ellipsis and one can do a --verbose to expand.

@hallyn
Copy link
Contributor

hallyn commented Mar 8, 2024

Sorry I'm not very familiar with what trivy provides. Can someone show me what will be the result if a container image has, let's say, an oci layout (inside the container image) containing a layer that has an initrd, which has a jar file which contains a vulnerable openssl?

@rchincha
Copy link
Contributor

Pending PRs both for zui and zli have been merged. Closing this issue.

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

No branches or pull requests

4 participants