Sails: Command Injection

[dev2games]

Sails version: 1.02
Node version: 9.2.0
NPM version: 6.3.0
DB adapter name: N/A
DB adapter version: N/A
Operating system: Ubuntu 16.04

---

Running NPM Audit throws this critical issue

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Critical │ Command Injection │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ open │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ No patch available │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sails │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ sails > sails-generate > machinepack-process > open │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://nodesecurity.io/advisories/663 │
└───────────────┴──────────────────────────────────────────────────────────────┘

[sailsbot]

@dev2games Thanks for posting, we'll take a look as soon as possible.

---

For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

[oaksofmamre]

This is a security vulnerability in sails-generate, meaning that you'd be able to hijack your own computer when running sails new. This one doesn't expose any tangible vulnerability to production applications. We will fix in future releases. Thanks so much for bringing this up @dev2games!

[hakash]

Hello,

This vulnerability led me here too, and it would seem like it it not only in sails-generate, but also in sails itself:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Critical      │ Command Injection                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ open                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sails                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sails > machinepack-process > open                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/663                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

The problem is fixed in the newest versjon of machine-pack (v4.x) where it has migrated to using opn as the replacement for open, which is deprecated.

[matdombrock]

I'm also getting this issue after install sails-mysql.

It seems to be documented in issue #4402 as well.

Sails version: 1.1.0
Node version: 8.15.0
NPM version: 6.4.1
DB adapter name: sails-mysql
DB adapter version: 1.0.1
Operating system: Ubuntu 18.04

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Critical      │ Command Injection                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ open                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sails                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sails > machinepack-process > open                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/663                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Critical      │ Command Injection                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ open                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ sails                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ sails > sails-generate > machinepack-process > open          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/663                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

So am I seeing that this is not actually a critical issue and can be safely ignored for now?

[hakash]

@oaksofmamre @raqem I've added a PR in 'sails-generate' to bump the 'machinepack-process' version to the latest, as both my manual test, and the automatic tests show no breaking changes between v2.0.2 and v4.0.0 for sails. Aparently it is only used when generating a new sails app ('sails new') to run the 'npm install' command after the files and directories are generated.

This should fix both critical vulns reported by 'npm audit'

[PavanBahuguni]

Is there a timeline when this can be fixed? I see there is already a PR by @hakash to fix the vulnerabilities
https://github.com/balderdashy/sails-generate/pull/35

[johnabrams7]

@dev2games @oaksofmamre @hakash @matdombrock @PavanBahuguni - Sails is currently working on the patch for this 👍

[mikermcneil]

@dev2games @hakash @matdombrock @PavanBahuguni In addition, please see https://github.com/balderdashy/sails/issues/4699#issuecomment-483829719 and https://github.com/balderdashy/sails/issues/4402#issuecomment-483869476.

(#4699, #4402)

[sailsbot]

Oh hey again, @dev2games. Now that this issue is reopened, we'll take a fresh look as soon as we can!

---

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

[mikermcneil]

@wulfsolter it's because I accidentally reopened it while logged in as sailsbot (I realized after closing it it'd be better to wait until we're through with our current purge before closing these vulnerability-related issues)

Also re the new message: sailsbot's new MO is that instead of parroting the same thing she says on initial opening, she responds to any reopening of issues and PRs with a shorter, sweeter message (mainly just to remind folks of the two points in the little footer thingie)

[johnabrams7]

@dev2games @hakash @matdombrock @PavanBahuguni Multiple related PRs for this were merged a day ago - how is the behaviour now? I welcome the rest of the community to test this out as well 👍