search

OpenZeppelin / openzeppelin-sdk

OpenZeppelin SDK repository for CLI and upgrades.js. No longer actively developed.
MIT License
432 stars 200 forks source link

npm audit reports critical vulnerability when @openzeppelin/cli is installed #1550

Open shark0der opened 4 years ago

shark0der commented 4 years ago

image

I've skimmed through the source, I wouldn't say you're affected but you'd probably want to remove the dependency and implement those functions yourself

Advisory: https://npmjs.com/advisories/1507

Usages

  1. https://github.com/OpenZeppelin/openzeppelin-sdk/blob/c68234271a4a6e29b989fe1bac1902a246a298b3/packages/cli/src/models/dependency/Dependency.ts#L58
  2. https://github.com/OpenZeppelin/openzeppelin-sdk/blob/578e4f27702d66dcbc23292950a221ea00b008c8/packages/cli/src/models/config/TruffleConfig.ts#L54

Looking at the dependency itself, I'd suggest implementing exec commands in your sdk and getting rid of the dep as it doesn't seem to be actively maintained.

Dep source: https://github.com/Manak/npm-programmatic/blob/master/index.js

Initially reported on Telegram: https://t.me/zeppelinos/12443

abcoathup commented 4 years ago

Hi @shark0der!

Thanks so much for reporting it! The project owner will review and triage this issue during the next week.

frangio commented 4 years ago

Thanks for reporting @shark0der. I looked into this and I don't think it's a vulnerability for OpenZeppelin CLI users, because the places where there could be command injection in the user's machine are controlled by the user.

shark0der commented 4 years ago

Hi @frangio! Indeed, like I said before, it doesn't seem like there's an impact (unless of course there are specific scenarios for some users), however, having npm audit report a critical vulnerability after the installation doesn't look pretty and might scare some users. That being said, fixing this doesn't need a very high priority but it would be nice to have it fixed sooner than later.