7 ms·

Netmask NPM package, used by 270k+ projects, vulnerable to octal input data

123 points by chha 6y ago

jojobas 6y ago
NPM was a mistake. Unvetted code essentially marketed as an extended standard library.
- mdpopescu 6y ago
  I think this is another example of "worse is better" [1] -- it's something that's yucky, and has a lot of flaws, but worked (for some value of worked) well enough for a lot of people. [1] https://www.dreamsongs.com/WorseIsBetter.html https://www.dreamsongs.com/WorseIsBetter.html
Cthulhu_ 6y ago
This is one reason why a company like NPM (or whoever is behind it), or really any company that hosts dependencies, should audit and verify packages and package updates. I'm sure there's businesses that would pay for a dependency provider that ensures all versions of all packages hosted there are reviewed, security checked and signed off on. With a warranty clause, so that if something like this does come out, they get compensated for damages (if actively exploited). A bug bounty should of course also be offered. A bit like Apple's app store but for libraries. Or the ideal thereof anyway. Basically a library developer can't just keep spamming updates, they would have to be more careful with what they submit. And of course, library devs would get a slice of the pie, an X amount per installation. It probably wouldn't work because people (even large enterprises who cannot afford any security issue like this) prefer free.
- tpetry 6y ago
  This is a bug in library, something the develops did not know off. When someone has to audit every package first there will be almost no open source anymore. The real problem is that the standard library of node/javascript is too small so you need dozens of packages for basic operations.
- hashkb 6y ago
  > whoever is behind it That would be Microsoft.
- raesene9 6y ago
  No one (that I'm aware of) has managed to make a successful business of "curated" packages (interested to hear if there are any). You get companies doing things like version checking and basic scans for malware, but that wouldn't catch issues like this. My guess is that the volume of security review you'd need to do, for it to be usable by enough companies, is massive and a lot of the review would need to be manual. Then as you add libraries you need to dedicate resources to reviewing every new release.
  - verall 6y ago
    What is a Linux distro
    - raesene9 6y ago
      As far as I'm aware, distros are not doing security review of all the packages they include. I'd be pretty surprised to find out that they have the kind of resources needed to do code review of even a small percentage of their packages.
  - jakelazaroff 6y ago
    Most packages don’t have 3 million weekly downloads, though. Surely Microsoft has the resources to review at least some of the most popular ones.
    - raesene9 6y ago
      So the challenge even for someone like MS is, how far down the line do they go. Do they do a one-time review, problem there is it won't catch new issues. Do they provide PRs for fixing issues found, that's another level of complexity. If the library owners don't patch promptly, do MS fork the lib, if they do, will people move to their forked version? It's a complex area to say the least :) It'd be great to see more companies take it on but I have a feeling it'll be a long time before we see significant coverage of this kind of issue.
      - 6y ago
- bionhoward 6y ago
  sounds extremely difficult
- hakcermani 6y ago
  It (the audit) could start with packages that have a lot of dependency. or the deepest dependency chain ?
tpetry 6y ago
Tldr: Leading zeros in ips are not calculated correctly. The stated „server-side request forgery, remote file inclusion, local file inclusion, and more“ issues are not within the package, these are anticipated issues when ips pass the validation which should fail...
- dahfizz 6y ago
  Not quite. The issue is that _valid_ IPs expressed in octal are being accepted and parsed in decimal.
rwmj 6y ago
Isn't this good? It'll be fixed in one place, and 270k other projects will get the fix, more or less automatically? Bugs happen, I'd rather they happen this way than the code had been copied into the other projects.
- epmaybe 6y ago
  I think the problem is that people (myself included) rarely update npm packages, and perhaps the upstream creators of some libraries won’t update either, mainly out of fear the update will break some part of your project.
  - rwmj 6y ago
    Sure, but there is a chance you will update them, whereas if you'd homebrewed the code or copied the code in, the bug would probably never be detected or fixed. And for those who do care about their packages and deps, they update in a timely manner.
  - yjftsjthsd-h 6y ago
    Just stick a `npm audit` in your CI jobs to run weekly or so?
- hashkb 6y ago
  The decade of vulnerability will never be addressed.
  - rwmj 6y ago
    But the alternative is the code was copied in or written from scratch, and that's worse.
    - hashkb 6y ago
      There are other alternatives. As mentioned elsewhere in the comments, there are other environments, like Ruby and Python, that don't seem to have these issues with the same severity.
qwertox 6y ago
NPM scared me right from the beginning. There was so much stuff getting pulled in, that it was pretty clear that this can't be secure. This is what mostly kept me away from node. I have the same feeling about PyPI, but to a way lesser extent. Python can already do much by itself, so that the amount of packages one installs is controllable.
- golergka 6y ago
  What's the fundamental difference between code that is written as a part of the standard library and the code that is a part of a package? Both are open-sourced and are written by pretty much the same people.
  - johannes1234321 6y ago
    My hope is that with a larger standard library there is more shared ownership of the library at large. With many small libraries there often is a single person working on it and others assuming "oh, somebody will have reviewed it" This can happen with a big library as well (as individual maintainers "own" specific parts) but there it is easier to establish a review culture. (Whether a review would have caught this specific case is yet another question, but again having an organisation could guarantee a timely response, is read if depending on a single contributor)
  - mekkkkkk 6y ago
    Some amount of oversight by qualified people?
  - jadedtuna 6y ago
    I would say the difference is, standard library goes through mode scrutiny/checks before an update is released. This of course depends on the library in question, as some non-std libraries are certainly subjected to more checks than others.
  - mikepurvis 6y ago
    The standard libraries receive far, far more review, both at the API design and implementation level. Anyone can push literally anything to PyPI.
sumanthvepa 6y ago
A large number of packages may be using it but the actual danger posed is very much dependent on the application using the library. That's a little hard to assess.
- stefan_ 6y ago
  This is nodes contribution to the world - now every npm package bug is hyped as a security issue instead of, well, this IP input field doesn't handle octal IPs correctly. Bonus points for security scanners that diagnose grave vulnerabilities in frontend bundles but the backend is some Python/Ruby/guaranteed-no-npm API.
  - Vinnl 6y ago
    The same would hold true if it was a single function part of a big single-package framework though.
phsau 6y ago
Can anyone explain a scenario where this is exploitable? I don't understand how this is likely to result in SSRF, file inclusion, etc. What applications are taking user supplied IP addresses?
- raesene9 6y ago
  So one example would be. Say you have a web application which makes HTTP requests on your customer's behalf (something like a website availability checker) You want customers to be able to enter host IPs that they control and your site will retrieve the URL on that site that they specify to confirm it's available. You don't want customers to be able to request things like http://127.0.0.1:8080 http://127.0.0.1:8080 or http://192.168.1.1:6443 http://192.168.1.1:6443 as you've got internal systems running there that are not for external use. So in your code you set the internal only ranges to be blocked. If you used this library to do that, it would be possible to bypass the restriction and request internal IPs by using octal encoding, as the customer could enter an octal IP and then the conversion would allow for ranges that should be blocked, to be requested.
  - phsau 6y ago
    Thanks, that makes sense. I was thinking along the lines of applications interpreting HTTP headers like X-Forwarded-For, but trusting the client (instead of re-writing).
  - yencabulator 6y ago
    So now the attacker just needs `evil.example.com A 127.0.0.1` in DNS. The protection has to sit in the HTTP client library after resolving the hostname to IP address, before connecting.
fanf2 6y ago
This bug is not NPM’s nor JavaScript’s fault. It’s mostly due to bad specifications of how IPv4 addresses are represented in text. The writeup at https://www.bleepingcomputer.com/news/security/critical-netmask-networking-bug-impacts-thousands-of-applications/ https://www.bleepingcomputer.com/news/security/critical-netm... has a link to an informative ancient expired IETF draft https://tools.ietf.org/html/draft-main-ipaddr-text-rep https://tools.ietf.org/html/draft-main-ipaddr-text-rep which describes how things stood in 2003. A more recent and more official RFC from the IETF on the security implications of inconsistent parsers discusses the issue behind this CVE https://tools.ietf.org/html/rfc6943#section-3.1.1 https://tools.ietf.org/html/rfc6943#section-3.1.1 The problem is that for a very long time the IETF did not specify the textual syntax of IPv4 addresses, and the POSIX specification for parsing dotted quads is bonkers. I think it is unfortunate that they fixed the bug by aligning with inet_aton()’s ancient foolish support for octal, instead of inet_pton()’s newer strict decimal syntax, forbidding leading zeroes.
- jtdev 6y ago
  Based on your comment, should we expect this vulnerability in comparable packages from other languages/communities? Or is this an NPM/JS maturity issue, i.e., “it’s not my fault, fix the spec.”?
  - FDSGSG 6y ago
    This is a super common bug.
  - fanf2 6y ago
    It’s obscure enough that I would expect this bug to turn up elsewhere, even though it’s an example of a fundamental security risk of parsers that don’t accept the same syntax (http://langsec.org/ http://langsec.org/), and IPv4 addresses are a specific instance of this risk that has been written about for years. (Tho usually the weirdness of inet_aton() is treated as quirky rather than dangerous.) I like the twist in the vulnerability report, using differences between IPv4 parsers to get past protections against things like SSRF, which I don’t think is explicitly mentioned in the IETF draft and RFC that I linked to.
jusssi 6y ago
The random thing learned from the article, is that ping parses octal, and apparently also hexadecimal. So now I can do ping 0x7f.0x1 next time I want to show off.
- pajko 6y ago
  Actually it does more than that. Try pinging 64465365 or 127.7575
  - Arnavion 6y ago
    (Recent related submission: https://news.ycombinator.com/item?id=25545967 https://news.ycombinator.com/item?id=25545967 )
- justinsaccount 6y ago
  ping does not, glibc does.
lucumo 6y ago
Some people will use this to argue against dependencies or npm, but I would argue that this vulnerability is actually a great example of why those are good things. The bug is caused by an esoteric IP address notation. Nearly everyone who would need this functionality would get the implementation wrong. In the exact same way the author of netmask did. However, netmask is now fixed and this bug will now no longer appear in any new implementation. Even better: npm will start to give warnings about this vulnerable package and will provide a fix through npm audit. Everything that is maintained in a reasonable way, will now no longer have this vulnerability. This is not a failure of dependencies or npm: this is an example why those are good things.
- selykg 6y ago
  Pros and cons. Everything has them. I think the really big issue with all these package management solutions is how deep the dependency requirements go. One package depends on another package that depends on 18 other packages, that depend on 30 other packages, and it's packages all the way down. No one is going to review all of those packages regularly. It's just not going to happen. Sure, it's less a problem for the big popular packages, but those big popular packages still depend on a lot of smaller packages and those smaller packages are extremely vulnerable due to sheer lack of eyes on it. Everyone always says "I won't use software that isn't open source because the code is in the open and vulnerabilities can be found" but the fact of the matter is someone has to be reviewing that code to find them. I'd love to better understand how many people are out there reviewing code as part of their process. I'll be it's a lot less than anyone would like.
  - mason55 6y ago
    The alternative is to develop the entire stack in house. Unlikely that you’ll do any better and meanwhile your competitors will pass you by. It’s really just the consequence of asymmetrical outcomes. Most of the time it all just works and so if you don’t keep up you’re out of business. When something goes wrong the whole world is exposed. I guess with npm you do have the additional wrinkle that much of it is outside the std lib which increases risk a bit.
the_duke 6y ago
Tangentially related: Many companies have auditing requirements for external dependencies, with increasing strictness for more sensitive domains. It would be immensely helpful to distribute this effort. We could have a platform that pays top domain experts and security researchers for audits. Companies can get access to via a subscription model or by paying for specific dependencies. Vulnerabilities would also be reported and fixed, helping everyone, and companies benefit by having a trustworthy source for audits and save internal work. Ideally the platform would be successful enough to open up a good amount of audits publicly to benefit the whole community. Also related: cargo-crev [1] explores a concept for shared auditing and trust for Rust crates. [1] https://github.com/crev-dev/cargo-crev https://github.com/crev-dev/cargo-crev
- rozab 6y ago
  Either the bugs aren't reported to upstream and it remains vulnerable, or they are reported and get fixed, making paying to access the audit pointless. The first option would basically involve the platform maintaining a closed fork, which won't work with most licenses and would be terrible for the ecosystem.
  - the_duke 6y ago
    I don't see the connection. As I said, many companies require a review/sign off for each dependency (version). Since they usually can't get them externally, they have do it internally. Enterprises also throw a lot of money at support contracts they never use, there is absolutely space for auditing expenses that give assurance and will be more thorough than any internal process.
tyingq 6y ago
Octects can be octal, hah!
hospadar 6y ago
I'm really curious - has anyone ever worked somewhere where _all_ packages are _actually_ reviewed? I'm pretty sure this really is a thing in some places (defense contractors, etc), but I'm curious if anyone's actually been exposed to a real company that really reviews everything.
- Leherenn 6y ago
  In those places, the reality is that the vast majority of "packages" will be coded in house.
toxik 6y ago
I feel like the proper fix is to NOT have such a versatile IP address interpretation in browsers etc.
skrebbel 6y ago
Sidenote: whoever thought octal notation was useful, was, well, wrong. Can we just stop supporting it entirely? I mean, how much of the internet will break if octal IPv4 addresses would stop working? Or if `010` would stop compiling in C sources except if some --enable-octal flag were supplied?
richrichardsson 6y ago
> if byte.length > 2 and (byte[1] == 'x' or byte[1] == 'x') Am I going crazy or is the `or` clause completely pointless?
- minitech 6y ago
  One of them is uppercase in the actual code.
  - 0xdba 6y ago
    https://github.com/rs/node-netmask/commit/ec1b5b56a822babf90e1e14d7fc8a56e06ba9013#diff-04f4f5b6e6cd99b4a664143f590d40d005f9b658d4ae0678ddc25c49bed19e2bR13 https://github.com/rs/node-netmask/commit/ec1b5b56a822babf90... I don't see it.
    - minitech 6y ago
      Ah, sorry, it was fixed in https://github.com/rs/node-netmask/commit/3f19a056c4eb808ea4a29f234274c67bc5a848f4#diff-04f4f5b6e6cd99b4a664143f590d40d005f9b658d4ae0678ddc25c49bed19e2b https://github.com/rs/node-netmask/commit/3f19a056c4eb808ea4.... And good catch, of course :)
rozab 6y ago
I wonder what the average dependency tree depth is for these 270k projects? How long will it take for the fix to propagate to the majority of them?
koolba 6y ago
IIUC, this is not exploitable in the more common setups where the IP address is being sourced from the inbound request. It's only an issue when the IP address itself is supplied by the user as text. Using something like the "ip" field on an inbound request would not be vulnerable.
GuidoW 6y ago
The problem is that the input is parsed twice. First by the netmask function that reads 0127 as 127 and the second time by the js-network stack code that reads it differently. The solution is not to change netmask to ignore leading zeros. The solution is to parse it into 4 uint8 values, validate the netblock on the numeric values and if the range is approved, generate the ip-address from your four numbers. That way you know for sure that the js-network stack is going to interpret it as you intend. There are some beautiful (horrifying) examples in this presentation: https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-New-Era-Of-SSRF-Exploiting-URL-Parser-In-Trending-Programming-Languages.pdf https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-Ne...

123 points by chha 6y ago

jojobas 6y ago

NPM was a mistake. Unvetted code essentially marketed as an extended standard library.

mdpopescu 6y ago
I think this is another example of "worse is better" [1] -- it's something that's yucky, and has a lot of flaws, but worked (for some value of worked) well enough for a lot of people. [1] https://www.dreamsongs.com/WorseIsBetter.html https://www.dreamsongs.com/WorseIsBetter.html

Cthulhu_ 6y ago

This is one reason why a company like NPM (or whoever is behind it), or really any company that hosts dependencies, should audit and verify packages and package updates. I'm sure there's businesses that would pay for a dependency provider that ensures all versions of all packages hosted there are reviewed, security checked and signed off on. With a warranty clause, so that if something like this does come out, they get compensated for damages (if actively exploited). A bug bounty should of course also be offered. A bit like Apple's app store but for libraries. Or the ideal thereof anyway. Basically a library developer can't just keep spamming updates, they would have to be more careful with what they submit. And of course, library devs would get a slice of the pie, an X amount per installation. It probably wouldn't work because people (even large enterprises who cannot afford any security issue like this) prefer free.

tpetry 6y ago
This is a bug in library, something the develops did not know off. When someone has to audit every package first there will be almost no open source anymore. The real problem is that the standard library of node/javascript is too small so you need dozens of packages for basic operations.
hashkb 6y ago
> whoever is behind it That would be Microsoft.
raesene9 6y ago
No one (that I'm aware of) has managed to make a successful business of "curated" packages (interested to hear if there are any). You get companies doing things like version checking and basic scans for malware, but that wouldn't catch issues like this. My guess is that the volume of security review you'd need to do, for it to be usable by enough companies, is massive and a lot of the review would need to be manual. Then as you add libraries you need to dedicate resources to reviewing every new release.
- verall 6y ago
  What is a Linux distro
  - raesene9 6y ago
    As far as I'm aware, distros are not doing security review of all the packages they include. I'd be pretty surprised to find out that they have the kind of resources needed to do code review of even a small percentage of their packages.
- jakelazaroff 6y ago
  Most packages don’t have 3 million weekly downloads, though. Surely Microsoft has the resources to review at least some of the most popular ones.
  - raesene9 6y ago
    So the challenge even for someone like MS is, how far down the line do they go. Do they do a one-time review, problem there is it won't catch new issues. Do they provide PRs for fixing issues found, that's another level of complexity. If the library owners don't patch promptly, do MS fork the lib, if they do, will people move to their forked version? It's a complex area to say the least :) It'd be great to see more companies take it on but I have a feeling it'll be a long time before we see significant coverage of this kind of issue.
    - 6y ago
bionhoward 6y ago
sounds extremely difficult
hakcermani 6y ago
It (the audit) could start with packages that have a lot of dependency. or the deepest dependency chain ?

tpetry 6y ago

Tldr: Leading zeros in ips are not calculated correctly. The stated „server-side request forgery, remote file inclusion, local file inclusion, and more“ issues are not within the package, these are anticipated issues when ips pass the validation which should fail...

dahfizz 6y ago
Not quite. The issue is that _valid_ IPs expressed in octal are being accepted and parsed in decimal.

rwmj 6y ago

Isn't this good? It'll be fixed in one place, and 270k other projects will get the fix, more or less automatically? Bugs happen, I'd rather they happen this way than the code had been copied into the other projects.

epmaybe 6y ago
I think the problem is that people (myself included) rarely update npm packages, and perhaps the upstream creators of some libraries won’t update either, mainly out of fear the update will break some part of your project.
- rwmj 6y ago
  Sure, but there is a chance you will update them, whereas if you'd homebrewed the code or copied the code in, the bug would probably never be detected or fixed. And for those who do care about their packages and deps, they update in a timely manner.
- yjftsjthsd-h 6y ago
  Just stick a `npm audit` in your CI jobs to run weekly or so?
hashkb 6y ago
The decade of vulnerability will never be addressed.
- rwmj 6y ago
  But the alternative is the code was copied in or written from scratch, and that's worse.
  - hashkb 6y ago
    There are other alternatives. As mentioned elsewhere in the comments, there are other environments, like Ruby and Python, that don't seem to have these issues with the same severity.

qwertox 6y ago

NPM scared me right from the beginning. There was so much stuff getting pulled in, that it was pretty clear that this can't be secure. This is what mostly kept me away from node. I have the same feeling about PyPI, but to a way lesser extent. Python can already do much by itself, so that the amount of packages one installs is controllable.

golergka 6y ago
What's the fundamental difference between code that is written as a part of the standard library and the code that is a part of a package? Both are open-sourced and are written by pretty much the same people.
- johannes1234321 6y ago
  My hope is that with a larger standard library there is more shared ownership of the library at large. With many small libraries there often is a single person working on it and others assuming "oh, somebody will have reviewed it" This can happen with a big library as well (as individual maintainers "own" specific parts) but there it is easier to establish a review culture. (Whether a review would have caught this specific case is yet another question, but again having an organisation could guarantee a timely response, is read if depending on a single contributor)
- mekkkkkk 6y ago
  Some amount of oversight by qualified people?
- jadedtuna 6y ago
  I would say the difference is, standard library goes through mode scrutiny/checks before an update is released. This of course depends on the library in question, as some non-std libraries are certainly subjected to more checks than others.
- mikepurvis 6y ago
  The standard libraries receive far, far more review, both at the API design and implementation level. Anyone can push literally anything to PyPI.

sumanthvepa 6y ago

A large number of packages may be using it but the actual danger posed is very much dependent on the application using the library. That's a little hard to assess.

stefan_ 6y ago
This is nodes contribution to the world - now every npm package bug is hyped as a security issue instead of, well, this IP input field doesn't handle octal IPs correctly. Bonus points for security scanners that diagnose grave vulnerabilities in frontend bundles but the backend is some Python/Ruby/guaranteed-no-npm API.
- Vinnl 6y ago
  The same would hold true if it was a single function part of a big single-package framework though.

phsau 6y ago

Can anyone explain a scenario where this is exploitable? I don't understand how this is likely to result in SSRF, file inclusion, etc. What applications are taking user supplied IP addresses?

raesene9 6y ago
So one example would be. Say you have a web application which makes HTTP requests on your customer's behalf (something like a website availability checker) You want customers to be able to enter host IPs that they control and your site will retrieve the URL on that site that they specify to confirm it's available. You don't want customers to be able to request things like http://127.0.0.1:8080 http://127.0.0.1:8080 or http://192.168.1.1:6443 http://192.168.1.1:6443 as you've got internal systems running there that are not for external use. So in your code you set the internal only ranges to be blocked. If you used this library to do that, it would be possible to bypass the restriction and request internal IPs by using octal encoding, as the customer could enter an octal IP and then the conversion would allow for ranges that should be blocked, to be requested.
- phsau 6y ago
  Thanks, that makes sense. I was thinking along the lines of applications interpreting HTTP headers like X-Forwarded-For, but trusting the client (instead of re-writing).
- yencabulator 6y ago
  So now the attacker just needs `evil.example.com A 127.0.0.1` in DNS. The protection has to sit in the HTTP client library after resolving the hostname to IP address, before connecting.

fanf2 6y ago

This bug is not NPM’s nor JavaScript’s fault. It’s mostly due to bad specifications of how IPv4 addresses are represented in text. The writeup at https://www.bleepingcomputer.com/news/security/critical-netmask-networking-bug-impacts-thousands-of-applications/ https://www.bleepingcomputer.com/news/security/critical-netm... has a link to an informative ancient expired IETF draft https://tools.ietf.org/html/draft-main-ipaddr-text-rep https://tools.ietf.org/html/draft-main-ipaddr-text-rep which describes how things stood in 2003. A more recent and more official RFC from the IETF on the security implications of inconsistent parsers discusses the issue behind this CVE https://tools.ietf.org/html/rfc6943#section-3.1.1 https://tools.ietf.org/html/rfc6943#section-3.1.1 The problem is that for a very long time the IETF did not specify the textual syntax of IPv4 addresses, and the POSIX specification for parsing dotted quads is bonkers. I think it is unfortunate that they fixed the bug by aligning with inet_aton()’s ancient foolish support for octal, instead of inet_pton()’s newer strict decimal syntax, forbidding leading zeroes.

jtdev 6y ago
Based on your comment, should we expect this vulnerability in comparable packages from other languages/communities? Or is this an NPM/JS maturity issue, i.e., “it’s not my fault, fix the spec.”?
- FDSGSG 6y ago
  This is a super common bug.
- fanf2 6y ago
  It’s obscure enough that I would expect this bug to turn up elsewhere, even though it’s an example of a fundamental security risk of parsers that don’t accept the same syntax (http://langsec.org/ http://langsec.org/), and IPv4 addresses are a specific instance of this risk that has been written about for years. (Tho usually the weirdness of inet_aton() is treated as quirky rather than dangerous.) I like the twist in the vulnerability report, using differences between IPv4 parsers to get past protections against things like SSRF, which I don’t think is explicitly mentioned in the IETF draft and RFC that I linked to.

jusssi 6y ago

The random thing learned from the article, is that ping parses octal, and apparently also hexadecimal. So now I can do ping 0x7f.0x1 next time I want to show off.

pajko 6y ago
Actually it does more than that. Try pinging 64465365 or 127.7575
- Arnavion 6y ago
  (Recent related submission: https://news.ycombinator.com/item?id=25545967 https://news.ycombinator.com/item?id=25545967 )
justinsaccount 6y ago
ping does not, glibc does.

lucumo 6y ago

Some people will use this to argue against dependencies or npm, but I would argue that this vulnerability is actually a great example of why those are good things. The bug is caused by an esoteric IP address notation. Nearly everyone who would need this functionality would get the implementation wrong. In the exact same way the author of netmask did. However, netmask is now fixed and this bug will now no longer appear in any new implementation. Even better: npm will start to give warnings about this vulnerable package and will provide a fix through npm audit. Everything that is maintained in a reasonable way, will now no longer have this vulnerability. This is not a failure of dependencies or npm: this is an example why those are good things.

selykg 6y ago
Pros and cons. Everything has them. I think the really big issue with all these package management solutions is how deep the dependency requirements go. One package depends on another package that depends on 18 other packages, that depend on 30 other packages, and it's packages all the way down. No one is going to review all of those packages regularly. It's just not going to happen. Sure, it's less a problem for the big popular packages, but those big popular packages still depend on a lot of smaller packages and those smaller packages are extremely vulnerable due to sheer lack of eyes on it. Everyone always says "I won't use software that isn't open source because the code is in the open and vulnerabilities can be found" but the fact of the matter is someone has to be reviewing that code to find them. I'd love to better understand how many people are out there reviewing code as part of their process. I'll be it's a lot less than anyone would like.
- mason55 6y ago
  The alternative is to develop the entire stack in house. Unlikely that you’ll do any better and meanwhile your competitors will pass you by. It’s really just the consequence of asymmetrical outcomes. Most of the time it all just works and so if you don’t keep up you’re out of business. When something goes wrong the whole world is exposed. I guess with npm you do have the additional wrinkle that much of it is outside the std lib which increases risk a bit.

the_duke 6y ago

Tangentially related: Many companies have auditing requirements for external dependencies, with increasing strictness for more sensitive domains. It would be immensely helpful to distribute this effort. We could have a platform that pays top domain experts and security researchers for audits. Companies can get access to via a subscription model or by paying for specific dependencies. Vulnerabilities would also be reported and fixed, helping everyone, and companies benefit by having a trustworthy source for audits and save internal work. Ideally the platform would be successful enough to open up a good amount of audits publicly to benefit the whole community. Also related: cargo-crev [1] explores a concept for shared auditing and trust for Rust crates. [1] https://github.com/crev-dev/cargo-crev https://github.com/crev-dev/cargo-crev

rozab 6y ago
Either the bugs aren't reported to upstream and it remains vulnerable, or they are reported and get fixed, making paying to access the audit pointless. The first option would basically involve the platform maintaining a closed fork, which won't work with most licenses and would be terrible for the ecosystem.
- the_duke 6y ago
  I don't see the connection. As I said, many companies require a review/sign off for each dependency (version). Since they usually can't get them externally, they have do it internally. Enterprises also throw a lot of money at support contracts they never use, there is absolutely space for auditing expenses that give assurance and will be more thorough than any internal process.

tyingq 6y ago

Octects can be octal, hah!

hospadar 6y ago

I'm really curious - has anyone ever worked somewhere where _all_ packages are _actually_ reviewed? I'm pretty sure this really is a thing in some places (defense contractors, etc), but I'm curious if anyone's actually been exposed to a real company that really reviews everything.

Leherenn 6y ago
In those places, the reality is that the vast majority of "packages" will be coded in house.

toxik 6y ago

I feel like the proper fix is to NOT have such a versatile IP address interpretation in browsers etc.

skrebbel 6y ago

Sidenote: whoever thought octal notation was useful, was, well, wrong. Can we just stop supporting it entirely? I mean, how much of the internet will break if octal IPv4 addresses would stop working? Or if `010` would stop compiling in C sources except if some --enable-octal flag were supplied?

richrichardsson 6y ago

> if byte.length > 2 and (byte[1] == 'x' or byte[1] == 'x') Am I going crazy or is the `or` clause completely pointless?

minitech 6y ago
One of them is uppercase in the actual code.
- 0xdba 6y ago
  https://github.com/rs/node-netmask/commit/ec1b5b56a822babf90e1e14d7fc8a56e06ba9013#diff-04f4f5b6e6cd99b4a664143f590d40d005f9b658d4ae0678ddc25c49bed19e2bR13 https://github.com/rs/node-netmask/commit/ec1b5b56a822babf90... I don't see it.
  - minitech 6y ago
    Ah, sorry, it was fixed in https://github.com/rs/node-netmask/commit/3f19a056c4eb808ea4a29f234274c67bc5a848f4#diff-04f4f5b6e6cd99b4a664143f590d40d005f9b658d4ae0678ddc25c49bed19e2b https://github.com/rs/node-netmask/commit/3f19a056c4eb808ea4.... And good catch, of course :)

rozab 6y ago

I wonder what the average dependency tree depth is for these 270k projects? How long will it take for the fix to propagate to the majority of them?

koolba 6y ago

IIUC, this is not exploitable in the more common setups where the IP address is being sourced from the inbound request. It's only an issue when the IP address itself is supplied by the user as text. Using something like the "ip" field on an inbound request would not be vulnerable.

GuidoW 6y ago

The problem is that the input is parsed twice. First by the netmask function that reads 0127 as 127 and the second time by the js-network stack code that reads it differently. The solution is not to change netmask to ignore leading zeros. The solution is to parse it into 4 uint8 values, validate the netblock on the numeric values and if the range is approved, generate the ip-address from your four numbers. That way you know for sure that the js-network stack is going to interpret it as you intend. There are some beautiful (horrifying) examples in this presentation: https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-New-Era-Of-SSRF-Exploiting-URL-Parser-In-Trending-Programming-Languages.pdf https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-Ne...