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

Bundled JS files in benchmarks directory trigger vulnerability findings #792

Closed
dnnrly opened this issue Jan 14, 2025 · 3 comments
Closed

Comments

@dnnrly
Copy link

dnnrly commented Jan 14, 2025

Problem statement

Vulnerability scanning tools are identifying CVEs in Javascript files, in the benchmarks directory of the project, even though they are cannot be used by projects that import this package as a dependency.

Analysis

The minify package that is published to Go proxies contains Javascript files from the benchmarks directory. Vulnerability scanners often work by downloading the zipped code from the package and scanning all of the code statically and checking the hashes of files against a database of known issues. The JS files under benchmarks have CVEs registered against them and therefore cause this package to be identified against those CVEs.

Possible solutions

  1. (Preferred) Rename the benchmarks directory to _benchmarks - causing the directory to be ignored by the Go tooling and therefore not included in the packaged zip on Go proxy servers
  2. Manually update the JS files to later versions - this doesn't solve the issue, just pushes the problem away until further CVEs are uncovered
  3. Adopt a JS package management framework to provide an easier path to upgrading JS dependencies - this again defers the problem but also adds complexity of pulling the JS files out to perform the benchmarking tests

Analysis

Options 2 & 3 both have the problem that someone needs to perform the dependency update steps to pull in a version of the dep that does not have the vulnerability. It is possible that a CVE is open and no fix is available.

Preventing the scanners from being triggered in the first place feels like the right solution.

@tdewolff
Copy link
Owner

Thanks for the analysis, I've pushed through your suggestion.

@dnnrly
Copy link
Author

dnnrly commented Jan 14, 2025

Wow, that was fast! I don't suppose it's possible to get an official release with this change? 😄

@tdewolff
Copy link
Owner

Sure, I've released v2.21.3 ;-)

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

No branches or pull requests

2 participants