search

jeremylong / Open-Vulnerability-Project

Java libraries for working with available vulnerability data sources (GitHub Security Advisories, NVD, EPSS, CISA Known Exploited Vulnerabilities, etc.)
Apache License 2.0
107 stars 30 forks source link

Current NVD API problems disclose memory leak problem in this lib #188

Closed Vampire closed 1 week ago

Vampire commented 1 week ago

Since the NVD API is having these problems right now, the dependency check Gradle task (version 9.1.0) fails with OutOfMemoryErrors.

It seems to me that those are caused by an endless recursion in this library, probably eating up more heap on each recursion:

Caused by: java.lang.OutOfMemoryError: Java heap space
  at org.apache.hc.client5.http.async.methods.SimpleBody.getBodyText(SimpleBody.java:86)
  at org.apache.hc.client5.http.async.methods.SimpleHttpResponse.getBodyText(SimpleHttpResponse.java:124)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:330)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  at io.github.jeremylong.openvulnerability.client.nvd.NvdCveClient.next(NvdCveClient.java:343)
  [...]
jeremylong commented 1 week ago

See https://github.com/jeremylong/DependencyCheck?tab=readme-ov-file#mandatory-upgrade-notice

Vampire commented 1 week ago

Oh, I see, thanks. But isn't the memory leak on retry still a problem? Or is it also fixed in the newer version?

jeremylong commented 1 week ago

There is a bit of bad recursion that can occur under error conditions. The original decision was to attempt the calls again because the API was unstable. However, some limiters should be put in place - if it keeps failing we should likely not keep trying again.

jeremylong commented 1 week ago

See https://github.com/jeremylong/Open-Vulnerability-Project/pull/189

Vampire commented 1 week ago

Well, that PR does not really solve the memory leak, it just limits the amount of retries, the memory leak persists. If there would not be a memory leak, at some point you would get a stack overflow error, so limiting the amount of retries might still be a good idea.

But maybe the memory leak itself should also be fixed? :-)

jeremylong commented 1 week ago

The only time you would hit the recursive calls to next() is under error conditions. Not much I can do to fix the NVD API stability. By capping the number of recursive attempts the issue is effectively solved. If you want to submit a PR to remove the recursion that'd be swell.