search

status-im / react-native-desktop-qt

A Desktop port of React Native, driven by Qt, forked from Canonical
1.22k stars 85 forks source link

Snyk Vulnerability: Command Injection #447

Open corpetty opened 5 years ago

corpetty commented 5 years ago

Snyk found a vulnerability in this repo that allows for command injection, so it is high severity.

Unfortunately, there isn't an immediately PR-able fix for it via Snyk so more investigation on our end has to take place.

Here is some more info on the problem at hand.

vkjr commented 5 years ago

Thanks for finding!

corpetty commented 5 years ago

CWE on this vuln: https://cwe.mitre.org/data/definitions/77.html

vkjr commented 5 years ago

Shelljs currently used in devDependencies section package.json, so it shouldn't appear in production. But that should be double-checked.

vkjr commented 5 years ago

@corpetty, @oskarth

Made some investigation:

shelljs used by react-native itself (under dev-dependencies in package.json) and by yeoman-generator (which is in dependencies section of package.json)

screen shot 2019-01-16 at 4 58 10 pm

Inside yeoman-generator vulnerable exec() method used in two places to get git info. (node_modules/yeoman-generator/lib/actions/user.js file), No user input processed.

screen shot 2019-01-16 at 4 57 12 pm screen shot 2019-01-16 at 4 57 24 pm

In react-native shelljs library used inside helper scripts that aren't included in result application js code:

screen shot 2019-01-16 at 4 55 22 pm

Implementation of react-native core and components doesn't use shelljs, so I think no user input can be passed and executed via vulnerable exec() function.

For me that looks like we have no problems with this security warning. Wdyt?

corpetty commented 5 years ago

I'm going to double check the vulnerability docs for this and see if there are other methods of calling it. If that checks out then we can close this.