search

DiegoZoracKy / convert-excel-to-json

Convert Excel to JSON, mapping sheet columns to object keys.
MIT License
290 stars 91 forks source link

NPM Audit Security Report - Prototype Pollution Detected by convert-excel-to-json #45

Open TheTechChild opened 4 years ago

TheTechChild commented 4 years ago

I have had to remove convert-excel-to-json from my enterprise project due to a security vulnerability that was detected by an npm audit command this morning. Please update your dependency of yargs-parser to a version that doesn't include this security vulnerability. Unfortunately I cannot keep packages that expose any kind of security vulnerability due to my company's requirement of being PCI compliant. So please fix the vulnerability so that those of us that have to convert excel sheets in a corporate environment can use this package again. Thank you! Screen Shot 2020-09-23 at 9 57 24 AM

DiegoZoracKy commented 4 years ago

Hi @IAmIndra thanks for bringing attention to this matter. Do you think you could be able to run the command npm audit fix and push a PR with the result?

https://docs.npmjs.com/cli/audit

TheTechChild commented 4 years ago

The package "requires manual review and could not be updated" Screen Shot 2020-09-23 at 10 15 01 AM

TheTechChild commented 4 years ago

Any update on this?

UNIDY2002 commented 4 years ago

I am also interested in this issue (as I ran yarn audit today and saw this note), and suspect that the reason lies in magicli. However, magicli was last published two years ago, and therefore this issue might not be that easy to fix.

It seems that cli support is a feature of this repo, so perhaps a possible solution is using a magicli-less method to implement cli support...?

DiegoZoracKy commented 4 years ago

Hi folks, sorry for the delay things are pretty rushed around here. The problem is not magicli itself but one of its dependencies. If we would try to follow a magicliless way we would soon or later end up with a similar problem but with another dependency. Although I'm also the owner of magicli I won't find time to fix its dependencies vulnerabilities right now. We could try to release a new version without any CLI at all, and then get back with its CLI feature in the future. What do you guys think?

UNIDY2002 commented 4 years ago

Great thanks for your kind reply and sorry for not having looked carefully into the problem.

I think this is a proper solution, as users preferring vulnerability-free dependencies can update their packages and get their requirements fulfilled, while users in need of the cli feature can choose to explicitly specify a previous version.

Miniland1333 commented 3 years ago

I've put up a PR (https://github.com/DiegoZoracKy/cliss/pull/1) to update the upstream dependency yargs-parser within convert-excel-to-json > magicli > cliss. With the update, it appears that all of the tests for cliss are all still passing.

Miniland1333 commented 3 years ago

FYI we are getting close on fixing the underlying dependency (https://github.com/DiegoZoracKy/cliss/pull/3). I just have to get the repo owner to approve those changes as a patch release.

Miniland1333 commented 3 years ago

I am having problems updating the dependents of Cliss/Magcli, including covert-excel-to-json, due to circular dependencies. Since these dependencies are below v1.0.0, using the carat ^ to float the versions does not work. If any of you have suggestions on how this mess can be resolved, please comment on https://github.com/DiegoZoracKy/magicli/pull/3. image

Muratcol commented 3 years ago

Any solution yet?

Miniland1333 commented 3 years ago

The repository owner has not been online in quite a while. In the event @DiegoZoracKy becomes responsive again, we will likely just simultaneously bump all these connected repositories to version 1.0.0 and go from there.