search

Rantanen / node-mumble

Mumble client in Node.js
MIT License
155 stars 48 forks source link

Update dependencies #114

Closed jeyemwey closed 5 years ago

jeyemwey commented 5 years ago

protobufjs is having a vulnerability in the version linked in this project.

Please update the version to a newer version that is not affected by this vuln (> 5.0.3).

Matching issue in my project (depends on node-mumble): https://github.com/jeyemwey/mumbleRadio/issues/3

Rantanen commented 5 years ago

Finally had time to have a look at this.

The vulnerability is not applicable to node-mumble. The vulnerability is related to parsing malicious .proto files. Node-mumble parses only the built-in mumble.proto.

Fixing it would require refactoring parts of node-mumble as the new protobufjs doesn't seem to provide a synchronous "load .proto files" API that I would much prefer for process startup (that's when we load our proto files).

I might do the update at some point, but as the issue is not critical, I'm somewhat reluctant to make major changes in the way node-mumble loads the proto files.

I was hoping to be able to whitelist that specific vulnerability for node-mumble but apparently that's not possible with npm. :|

jeyemwey commented 5 years ago

Version 5.0.3 still has synchronous file loading (at least, testing with "protobufjs": "^5.0.3", works for me). so bumping up the version to 5.0.3 should work and fix the security alert.

Rantanen commented 5 years ago

Good point. Updated to ^5.0.0, which should bring it up to 5.0.3 or later if available.

I had just gone with npm audit fix --force and didn't check how far that pushed the protobufjs.

Published 0.3.18 now.

There's still few reported vulnerabilities, but these are in dev packages, such as the jsdoc-to-markdown that we are using for documentation.