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

package.json should use fixed versions. #570

Open
MicahZoltu opened this issue Dec 14, 2021 · 6 comments · May be fixed by #723
Open

package.json should use fixed versions. #570

MicahZoltu opened this issue Dec 14, 2021 · 6 comments · May be fixed by #723

Comments

@MicahZoltu
Copy link
Contributor

solc-js/package.json

Lines 47 to 57 in ee86329

"dependencies": {
"command-exists": "^1.2.8",
"commander": "^8.1.0",
"follow-redirects": "^1.12.1",
"fs-extra": "^0.30.0",
"js-sha3": "0.8.0",
"memorystream": "^0.3.1",
"require-from-string": "^2.0.0",
"semver": "^5.5.0",
"tmp": "0.0.33"
},

To protect users against supply chain attacks, package.json should refer to explicit versions and version updates should be done intentionally rather than "when CI happens to build" or "when user happens to install the dependency".

It is worth noting that this does not protect users against transitive dependencies of this library, unless each of the dependencies also uses explicit versions in package.json. To more thoroughly protect users against supply chain attacks, only dependencies that also use explicit dependency versions should be used and the same for their dependencies etc.


Separately, I think it is worth discussing the use of npm shrinkwrap. This would make it so the developers of this library can do a one-time audit of their dependency tree and then commit and publish that so downstream users don't have to each do a full audit of solcjs's transitive dependencies every time they update or install it.

@chriseth
Copy link
Contributor

I was not aware that the dependencies grew that much. Maybe it would be good to also reduce the non-dev-dependencies to a minimal set. I don't think anybody ever does a full review of all the libraries.

@MicahZoltu
Copy link
Contributor Author

#361 is tracking the reduction of dependencies. It is down a long way from when that issue was opened, but I suspect more progress could be made if someone dug into it a bit.

I do wonder if some of those could be moved to devDependencies? For example, semver maybe is used by the build pipeline but not runtime?

@cameel
Copy link
Member

cameel commented Dec 14, 2021

Thanks for opening the discussion.

The biggest problem I see here is that if we fix these versions, we force any downstream project to use the same exact versions and prevent them from updating. IMO the library should be as agnostic to that as possible. I.e. if we know it won't work correctly with a specific version of a specific dependency, we should adjust the range in package.json to exclude that version but other than that we should adapt to what downstream wants to use.

I think that many downstream projects (e.g. Truffle or Hardhat) are much more proactive about keeping their dependencies up to date than we are and it might be be more of a nuisance than help for them. I think that hard-coding of specific versions would only work if we could commit to keeping them always up to date and we currently and I don't think we have bandwidth for that.

@MicahZoltu
Copy link
Contributor Author

A downstream user can override versions if they want. For everything cryptocurrency, I think there is very significant value in defaulting to "safe" and letting users override to "unsafe but convenient" if they want, which is what fixed versioning would give us.

In the vast majority of cases, there is no need to override because the libraries in question are used internally not externally (in fact, I think in this case 100% of the dependencies are internal). If there is a security vulnerability that needs patched a downstream user can submit a PR to solc to fix it and manually override in the meantime.

I don't think there is any need (or desire) for solc to be constantly bumping it's dependencies. If the code it depends on works, the only downside to using an old version is that a downstream user may get multiple copies of the dependency. This only matters for bundling for browser, which isn't a common usecase for solc, and even if it was there are tools for dealing with that.

@cameel
Copy link
Member

cameel commented Dec 14, 2021

I don't think there is any need (or desire) for solc to be constantly bumping it's dependencies.

That's the part that I have a problem with. This assumes that vulnerabilities are rare and that it's easy to stay at a version without any. With the insane number of transitive dependencies that npm packages pull in I think it's almost certain that there are some vulnerabilities in our dependency tree at any given time. Most of the time when I install something with lots of dependencies, I see a screen full of warnings about security bugs. I think that the only sane strategy with npm is to keep things as up to date as possible to get all the latest patches.

A downstream user can override versions if they want.

Is this really that simple? Honest question - last time I tried to do something like this it was very hacky but it was a very long time ago so maybe things changed for the better, I don't know. I'm just assuming that overriding versions is not easy to do cleanly (e.g. without having to manually edit package-lock.json) and that we're basically forcing downstream projects to carry multiple copies of dependencies if we use fixed versions. And I think multiple copies are still a problem - if you're carefully keeping your dependencies up to date, you don't want libraries to bring in older versions behind your back.

@MicahZoltu
Copy link
Contributor Author

The risks we are comparing here are security vulnerabilities in dependencies against supply chain attacks. I think to properly make this assessment, we would need to look at the dependencies of solcjs and determine which ones are likely to have a security vulnerability that would impact downstream users. I don't know the details of how solcjs works, but I would suspect that almost all attack vectors via dependencies would require the attacker compromise the contract compilation process of a project, in which case they can do far more harm much easier than by exploiting a solcjs dependency bug.

On the flip side, supply chain attacks are a real thing that happens on a semi-regular basis on NPM and any dependency can be the target of a supply chain attack regardless of how it is used in the build process.

Others may have more insight into the first issue, but my gut tells me that supply chain attacks are by far the more risky issue in this context.

A downstream user can override versions if they want.
I haven't had to do it in a while because usually I just submit a PR upstream and it gets integrated. However, I believe it just requires an NPM shrinkwrap file as a sibling to package.json, and that file can just contain the override details?

@cameel cameel added this to solc-js Jun 10, 2022
@cameel cameel moved this to Triage in solc-js Jun 10, 2022
@cameel cameel moved this from Triage to Design/Decide in solc-js Jun 10, 2022
@KillariDev KillariDev linked a pull request Dec 20, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Design/Decide
Development

Successfully merging a pull request may close this issue.

3 participants