npm audit critical findings

[oliverkane]

Just writing to let you know that the current NPM package reports this little gem has a critical security issue to patch.

results of npm audit

┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Critical      │ Command Injection                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ macaddress                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ minecraft-protocol                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ minecraft-protocol > uuid-1345 > macaddress                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/654                       │
└───────────────┴───────────────────────────────────────────────

[roblabla]

Thank you for reporting this!

I don't think this affects us. From the report: "For this vulnerability to be exploited an attacker needs to control the iface argument to the one method."

• uuid doesn't seem to use the iface argument: https://github.com/scravy/uuid-1345/blob/master/index.js#L59

• This is only a problem if you use mac-based UUIDs, which I don't think we do (Can someone confirm?)

Since this issue doesn't really affect us, I don't think we'll take any immediate step to fix it. There is a PR up in the macaddress repo that fixes it: scravy/node-macaddress#18.

I'll keep this issue open so that we don't forget about this potential footgun though, until a fix is commited upstream.

EDIT: If we do use UUID.v1() somewhere, we can fix the issue by passing false to the mac option, which will cause UUID.v1() to revert to a time-based UUID, instead of a "proper" mac-based UUID: https://github.com/scravy/uuid-1345/blob/master/index.js#L433

[rom1504]

There's the uuid module that also supports uuid 1345 but doesn't support parsing.
So uuid-1345 is currently the only usable uuid module on npm.

[roblabla]

uuid-1345 and macaddress both got an update which fixes the underlying vulnerability. We might want to bump the package.json version of uuid-1345 to 0.99.7 so that people get the secure version.

[rom1504]

after updating to 0.99.7 locally (and rerunning npm install), I'm still getting warning with npm audit. No idea how that works

[rom1504]

npm audit
                                                                                
                       === npm audit security report ===                        
                                                                                
[+] no known vulnerabilities found
    Packages audited: 1467 (665 dev, 11 optional)