npm audit vulnerability: High
drc-cnicholson opened this issue · 13 comments
A recent vulnerability in the "tar" package was found.
dynamodb-localhost > tar
Allows an "Arbitrary File Overwrite"
#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.
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!
@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
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.
@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!
@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.
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!
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"
}
@AshanFernando Is there anything we can help with to either merge or reject #45? Just looking for either resolution or next steps.
@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?
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.