Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Security audit fail due to fixed tar dependency #1912

Open
wbt opened this issue Apr 15, 2019 · 8 comments
Open

Security audit fail due to fixed tar dependency #1912

wbt opened this issue Apr 15, 2019 · 8 comments

Comments

@wbt
Copy link
Contributor

wbt commented Apr 15, 2019

Issue

Installing the latest truffle-contract produces security audit failures.

Steps to Reproduce

  1. Run npm uninstall truffle-contract
  2. Run npm install truffle-contract
  3. npm reports + truffle-contract@4.0.11

Expected Behavior

No more output after that.

Actual Results

npm reports that it found vulnerabilities in the installed package.
npm audit gives details, such as:

High Arbitrary File Overwrite
Package tar
Patched in >=4.4.2
Dependency of truffle-contract
Path truffle-contract > web3 > web3-bzz > swarm-js > tar.gz > tar
More info https://npmjs.com/advisories/803

It looks like this is fixed in web3 v1.0.0-beta.38, so the fix might be just updating the dependency to a later version (also via the truffle-interface-adapter dependency). However, I'm not sure how much else that might break so this is not a well-tested PR.

Environment

  • Operating System: Win 10 Pro
  • node version (node --version): 9.3.0
  • npm version (npm --version): 6.9.0
@eggplantzzz
Copy link
Contributor

@wbt There has been a big discussion concerning web3 over the past few months. We would love to use the most recent web3 but there have been some big breaking changes in the last ~14 releases and they are too big for us to keep up with at the moment.

Hopefully we will be able to get all these resolved in the near future. You can check this out to get some more of the story.

@frangio
Copy link
Contributor

frangio commented May 3, 2019

@eggplantzzz Any updates on this? All repositories depending on Truffle are getting security alerts.

@wbt
Copy link
Contributor Author

wbt commented May 3, 2019

@frangio There's an open PR by @eggplantzzz to upgrade web3 to the beta53, but as noted above there are some big breaking changes. There is at this time still unresolved discussion about how the version of Truffle incorporating that PR should be marked, to signal if users have to make changes in their code or not.

Truffle developers don't want to go up just one version in the 1.0.0-beta series (from 37 to 38) and deal with just the changes in that step, until they're making a big jump and dealing with all the breaking changes in all those intermediate steps, plus whatever other changes they've been waiting on for that next marker increment.

For now, we are supposed to just accept that advances in what is described as technology for secure immutable data and secure decentralized interaction will bring security problems that can't be quickly fixed due to bottleneck centralization in the development process & governance.

However, if you'd like to fix that bug in a fork, I'm guessing some security-sensitive developers might appreciate it!

@frangio
Copy link
Contributor

frangio commented May 3, 2019

It's certainly a possibility that everyone will begin using their own fork soon... I don't want that to happen, it should be fixed upstream, but I see that it's a complex situation.

@nivida
Copy link

nivida commented May 14, 2019

We do currently discuss the breaking changes in the issue web3/web3.js#2786 of web3.js. @frangio

@wbt
Copy link
Contributor Author

wbt commented Jun 12, 2019

@frangio Any update on if you want to move forward with developing a fork that uses web3-beta38 or otherwise fixes the issue? The on-hold status tag indicates we should not expect an "upstream" fix in Truffle anytime soon.

@eggplantzzz
Copy link
Contributor

eggplantzzz commented Jun 19, 2019

@wbt So I think we are still getting some of this web3 stuff straightened out and should have a plan for moving forward on this in the near future. Sorry for not being able to say more with certainty but hang tight!

@wbt
Copy link
Contributor Author

wbt commented Jul 3, 2019

For some additional context: this is a big thing that broke in beta.38, so just going up one version in the beta series does not seem to be a feasible solution even on a fork. That was fixed on Feb. 28 which went into beta.47.

Apparently the fix for this issue was part of a very large merge breaking other things too; small pull requests are not how things work in web3. There are too many commits in that PR for GitHub to display, making it a bit harder to isolate which of hundreds of commits to hundreds of files was the subset fixing this specific security issue.

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

No branches or pull requests

5 participants