sonatype / ossindex-public

Sonatype OSS Index - Public
Apache License 2.0
6 stars 9 forks source link

Externalreferences in practice appears to be (temporarily?) nullable, but is not marked as such #29

Closed aikebah closed 2 years ago

aikebah commented 2 years ago

In a comment on an OWASP DependencyCheck issue https://github.com/jeremylong/DependencyCheck/issues/3707#issuecomment-938024734 a NullPointerException surfaced when DependencyCheck was dereferencing the externalReferences of a ComponentReportVulnerability from a retrieved report for pkg:maven/com.thoughtworks.xstream/xstream@1.4.17. As other methods in the API are clearly marked as @Nullable this to me is an unexpected NullPointerException. If the API can (temporarily) return vulnerabilities with no external references the method should either be annotated with @Nullable or the getter should null-check and return an empty list for the null-case When (re)testing for the same library I could not reproduce. My assumption is that by the time I tried the affected vulnerability had been enriched by its external references. If the API is not expected to respond with null-valued externalReferences for any vulnerability there appears to be a transactional hole in between registering a vulnerability and its externalReferences that would allow the API to return invalid responses.

https://github.com/sonatype/ossindex-public/blob/27d7bac26b1673ea8ed07151cb34c084bb7d7e64/api/src/main/java/org/sonatype/ossindex/service/api/componentreport/ComponentReportVulnerability.java#L211-L216

ndonewar commented 2 years ago

Hi @aikebah - thanks for the bug report! I wasn't able to reproduce the issue locally, even by forcing the system to return zero external references. The API endpoint should always return that field, and do so with an empty list if necessary. There are internal tests that verify this case.

I agree though that it's better to add a null check in the getter, which also follows the existing pattern set in other methods not marked with @Nullable. The change that fixes this issue was just released as part of 1.8.1: https://github.com/sonatype/ossindex-public/releases/tag/release-1.8.1