search

kurttheviking / git-rev-sync-js

Synchronously get the current git commit hash, tag, or branch
MIT License
171 stars 58 forks source link

Fixes vulnerability in shelljs #34

Closed gl2748 closed 6 years ago

gl2748 commented 6 years ago

Vulnerability: https://github.com/shelljs/shelljs/issues/143

gl2748 commented 6 years ago

@kurttheviking thoughts on this?

kurttheviking commented 6 years ago

@gl2748 i havent had a chance to look yet; ill take a look soon. is this linked to a known cve?

gl2748 commented 6 years ago

@kurttheviking hi, yes this: https://snyk.io/vuln/npm:shelljs:20140723

kylemh commented 6 years ago

Still a vulnerability, even in 8.1

bnchdrff commented 6 years ago

i don't think there is any security issue here. this library could just use child process exec commands and it'd be just as risky, with the downside of harder to read code.

see https://github.com/kurttheviking/git-rev-sync-js/blob/master/index.js and look for invocations of shell via _command - none use dynamic input values.

kurttheviking commented 6 years ago

@gl2748 @bnchdrff executing shell commands is inherently risky but this package doesn't allow user input to modify system calls in any way. as a result, I'm closing this as wontfix unless someone has a clear path forward to avoid system calls at all.