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

npm audit vulnerability: High #43

Open
drc-cnicholson opened this issue Apr 11, 2019 · 13 comments
Open

npm audit vulnerability: High #43

drc-cnicholson opened this issue Apr 11, 2019 · 13 comments

Comments

@drc-cnicholson
Copy link

A recent vulnerability in the "tar" package was found.

dynamodb-localhost > tar

Allows an "Arbitrary File Overwrite"

https://nodesecurity.io/advisories/803

@sutt0n
Copy link
Contributor

sutt0n commented May 13, 2019

I opened a PR to update the tar package in #44

@YOU54F
Copy link

YOU54F commented May 15, 2019

Hi @AshanFernando,

Could we please release a new version of the NPM module please.

Many thanks!

@sutt0n
Copy link
Contributor

sutt0n commented May 15, 2019

@YOU54F / @AshanFernando:

#45 should be reviewed / merged when y'all get a chance. Since there were no unit tests, the tar update couldn't be properly verified. The PR includes the necessary code updates for https://github.com/npm/node-tar as well as a unit test or two.

@YOU54F
Copy link

YOU54F commented May 15, 2019

As a tester, it makes me very happy that you are adding unit tests ❤️ I'm just chasing a vulnerability in our dep chain, so just popped in to raise an issue and realised you had fixed it, so thought I would chase up for a release. I am sure the repo maintainers will be pleased!

@YOU54F
Copy link

YOU54F commented May 21, 2019

@AshanFernando - please can you update the NPM package, and bump the semver minor version please, when you are accepting PR's for vulnberabities, these should be published asap

@sesam
Copy link

sesam commented May 28, 2019

Maybe we can pay or donate to @AshanFernando or @99xt or something to speed this up?

UPDATE: I see a valid fix is already merged in #40 🥇 so we're only waiting for a minor version bump and publish. @AshanFernando do you have your npm credentials safe and sound? There was a reset due to the npm hacks not long ago, so you'll probably have to use your npm signup email to do a token reset before you can publish.

@YOU54F
Copy link

YOU54F commented May 29, 2019

@sesam - I appreciate the sentiment towards supporting OSS projects, however paying to get an npm package published to fix security vulnerabilities isn't really on. Especially when the vulnerabilties were fixed by the OSS community itself.

The repo owners, if responsible would update the package when they accept and merge a PR for a sec vuln.

I would much rather publish it with a fix, as a scoped package, and would advise the upstream users of the project to do so.

I appreciate that as an OSS package grows in adoption, it becomes a pain for an owner as they become responsible for maintaining it going forward, but they are not alone, as they is a whole OSS community wants to help, but if owners don't publish packages, then the ecosystem falls apart.

Look at lodash, and their out of date vulnerable modules and then unwillingness to update them, pull or mark vulnerable packages on npm as deprecated or vulnerable. It has affected security scanners which say every package using lodash is vulnerable even if they are using a modularised package that doesn't contain affected code!

@AshanFernando
Copy link
Collaborator

@sesame Feel extremely sorry for late catching up of these things. As the library has grown, it takes more time to verify the things. Also the unit tests has also broken, making more manual involement to test in windows, linux and mac. This is what takes more time and support. Since the library is used by many, small bug could create lots of issues for automations where we need to be very careful. I will check with few other maintainers to get this sorted within this week.

I completely agree with @YOU54F. This is not about donations. We need to empower the contributors and pave the way to absorb changes fast. Since the capacity of my self and few handful of maintainers is limited, we are planning for the following in near future;

  • Automate the NPM publishing process.
  • Including selected set of contributors as maintainers with authority to merge and publish.
  • Fix the unit tests which acts as an extra shield of safety.

@sesam
Copy link

sesam commented May 30, 2019

Great to hear! One more idea: how about keeping a tag or branch with the latest deployed code, to make it possible to publish a new package version that contains only the vulnerability fix. Even if there are several vulnerabilities or important fixes "in queue", sometimes it is a good idea to make a new version for each, because that also makes it easier for users to help with testing, because they can lock the package to any of the specific published versions. My $0.02 :) Have a great, productice day!

@buresmi7
Copy link

buresmi7 commented Jun 7, 2019

I was created fast hotifx #49.
Until this MR will be approved and new version will be released you can use our hotfixed fork:

"serverless-dynamodb-local/dynamodb-localhost": "git+https://github.com/Travelport-Czech/dynamodb-localhost.git#1049fe7c8dd5e8c92cd7fc1a4c7bb9b6a89b13a6"

if you use this package as dependency e.g. in package serverless-dynamodb-local use this in package.json:

"resolutions": {
    "serverless-dynamodb-local/dynamodb-localhost": "git+https://github.com/Travelport-Czech/dynamodb-localhost.git#1049fe7c8dd5e8c92cd7fc1a4c7bb9b6a89b13a6"
}

@matthopson
Copy link

@AshanFernando Is there anything we can help with to either merge or reject #45? Just looking for either resolution or next steps.

@waldoj
Copy link

waldoj commented Jun 19, 2019

@AshanFernando, what can I do to help you get #45 merged? It's been over two months since this vulnerability was reported, and we can see that there are nearly 800 GitHub projects that need this fix. Can I review the PR for you and document my findings? Can I build the project with this patch and report back with the results?

How can I help?

@alert-debug
Copy link

alert-debug commented Jun 7, 2021

The most recent version of this package, 0.0.9, depends on tar version ^4.4.8, which is not affected by the vulnerability in advisory 803. I think that means this issue can be closed.

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

9 participants