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

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

Closed
aikebah opened this issue Oct 7, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@aikebah
Copy link

aikebah commented Oct 7, 2021

In a comment on an OWASP DependencyCheck issue jeremylong/DependencyCheck#3707 (comment) 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.

/**
* @since 1.8.0
*/
public List<URI> getExternalReferences() {
return externalReferences;
}

@ndonewar
Copy link
Contributor

ndonewar commented Oct 8, 2021

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

@ndonewar ndonewar closed this as completed Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants