search

launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

Snyk vulnerability - URL Parse - Improper Input Validation #209

Closed naveentej closed 3 years ago

naveentej commented 3 years ago

https://snyk.io/vuln/SNYK-JS-URLPARSE-1078283

launchdarkly-node-server-sdk@5.13.3,launchdarkly-eventsource@1.3.1,original@1.0.2,url-parse@1.4.7

bwoskow-ld commented 3 years ago

Hi @naveentej,

Thanks for the vulnerability report. The launchdarkly-eventsource package currently uses the latest available original release (1.0.2). I see that original's maintainer has already updated their package dependencies for this issue on master but has not yet released this in a new version.

In the meantime, given that version 1.0.2 defines the url-parse version constraint as ^1.4.3, this version constraint is compatible with 1.5.0 and higher which contains the vulnerability resolution. As a result, this vulnerability shouldn't actually affect you -- when you install you should be able to get a patched url-parse version.

Cheers, @bwoskow-ld

eli-darkly commented 3 years ago

The 5.14.5 release changed the dependency declarations to explicitly exclude vulnerable versions of url-parse.

ryanpcmcquen commented 2 years ago

@eli-darkly

The 5.14.5 release changed the dependency declarations to explicitly exclude vulnerable versions of url-parse.

This still gets marked as a vulnerability by Snyk for 5.14.5.

Issues with no direct upgrade or patch:
  ✗ Improper Input Validation [High Severity][https://snyk.io/vuln/SNYK-JS-URLPARSE-2407770] in url-parse@1.5.3
    introduced by launchdarkly-node-server-sdk@5.14.5 > launchdarkly-eventsource@1.4.1 > original@1.0.2 > url-parse@1.5.3 and 1 other path(s)
  This issue was fixed in versions: 1.5.9

Is Snyk getting confused here?

eli-darkly commented 2 years ago

@ryanpcmcquen I think the answer is 1. it's talking about a different vulnerability, and 2. Snyk may be behaving as designed but I think the output is misleading.

The issue you're commenting on here is from over a year ago, when the latest 1.5.x version of url-parse did not have any known problems, and Snyk was complaining about a problem in 1.4.x. There are now known problems in all versions of url-parse below 1.5.9. So Snyk is correct in that sense.

However, I'm not sure why Snyk thinks our SDK has a dependency specifically on version 1.5.3 of url-parse. As described in the original response to this issue, the transitive dependency via original uses a loose version constraint with a ^ prefix, so the version of url-parse you actually get should be the latest 1.x version— currently 1.5.10.

My best guess is that Snyk is looking at the package-lock.json file that existed in the node-server-sdk repository at the time our version 5.14.5 was released. That reflects the version of url-parse that was the latest at that time. But, since package-lock.json is not used at all when NPM is resolving transitive dependencies, that is misleading. That's why in later releases of our SDK, we removed package-lock.json from source control; it doesn't really make sense to check in the lockfile for library code, as opposed to application code.

The 5.14.x version is almost at end of life, but it still has another three months of support, so it might make sense for us to put out a 5.14.6 patch that would make this warning go away. It wouldn't have any code changes at all, it just wouldn't have a lockfile.

ryanpcmcquen commented 2 years ago

The 5.14.x version is almost at end of life, but it still has another three months of support, so it might make sense for us to put out a 5.14.6 patch that would make this warning go away. It wouldn't have any code changes at all, it just wouldn't have a lockfile.

That would be great.

eli-darkly commented 2 years ago

@ryanpcmcquen We've released a 5.14.6 patch which addresses this.

ryanpcmcquen commented 2 years ago

@ryanpcmcquen We've released a 5.14.6 patch which addresses this.

Thank you!