8 ms·

20% of all Node.js modules found vulnerable to injection attacks

simonw 9y ago
"Vulnerable to injection attacks" seems like a bit of a stretch to me. Direct quote: "then about 20% of all modules turn out to directly or indirectly depend on at least one injection API" Where "injection API" is a reference to either the eval() or exec() functions.
- shawabawa3 9y ago
  To be fair they follow up with > A staggering 90% of the call sites do not use any mitigation technique at all. > Another 9% attempt to sanitise input using regular expressions. Unfortunately, most of those were not correctly implemented
bri3d 9y ago
This headline is entirely false - 20% of Node modules use 'eval' or 'exec'. While these are certainly less secure and a strong secure coding standard would probably ban them or at least reduce their use, it's entirely possible to use both in safe ways.
- tonyarkles 9y ago
  Further on: > 18,924 of all 51,627 call sites are found to be statically safe (36.66%) > The templates for the vast majority of call sites have at most one hole, and very few templates contain more than five. If you've got a dependency that calls eval or exec, those aren't great odds that they're doing it safely.
  - acdha 9y ago
    That still requires you to know whether either is being called with user-provided values before you can say it's unsafe. If, for example, someone had a utility which resized user-uploaded images you couldn't say simply calling exec to run something like ImageMagick was unsafe before checking whether it used the user's filename.
    - tonyarkles 9y ago
      I think that's what they're saying in those quoted lines though. That 36% appeared to be statically-safe (i.e. the string sent to eval or exec was generated using only strings defined in the code) and the others were not.
      - acdha 9y ago
        True — it wasn't clear to me whether e.g. a random temporary filename would be considered static or only actual fixed strings in the code. (Similar questions would arise if e.g. a web app saved things using a database primary key so it's technically not static but the attacker doesn't control it even if they control the file contents)
        tonyarkles 9y ago
        Ahhhhh nice, I hadn't considered the "random temporary filename" or "db primary key" aspect either. That's something that gives me a small chill, but it's not the end of the world compared to "arbitrary user input piped to exec()" Who knows how elaborate their static analyzer actually is in practice? I wrote a Python analyzer in grad school, and the results were both pretty interesting (type inference tracing through function calls) and pretty mediocre. It was also fiddly-as-hell to get working. Plus... the first rule of statically-analyzing dynamic languages is that the results get weird after the first eval()--anything could happen!
        acdha 9y ago
        Yeah, as a developer I see plenty of appeals to dynamic languages but they’re certainly a nightmare for tool authors. I know OpenStack was working on a security scanner for Python – I wonder what percentage of the open issues start with “this is probably very hard but…”
- ynniv 9y ago
  Possible, but unlikely. eval is particularly difficult to use correctly, and can almost always be replaced with something better.
- Yaggo 9y ago
  That 20% includes dependencies. I'm sure the majority of those are cli tools (webpack etc), i.e. they don't take unsanitized input from web forms etc.
- weej 9y ago
  Key word is vulnerable. There is a gap in protection and they are at risk to being potentially exploited.
  - weej 9y ago
    Doesn't mean it isn't click bait. ;-)
- Ajedi32 9y ago
  It's not even that. It's more like "20% of Node modules use _or include a module somewhere in their dependency chain which uses_ 'eval' or 'exec'". So if my backup utility happens to depend on a wrapper library for node's Child Process module (which probably doesn't sanitize its inputs, as that would break quite a bit of functionality), it's considered "vulnerable".
  - inetknght 9y ago
    And why shouldn't it be?
    - Ajedi32 9y ago
      Maybe because it's not sending user input into the "vulnerable" API? Or because it's not even calling the "vulnerable" API in the first place? Or because it's a system utility which will never be exposed to untrusted user input?
- ScottBurson 9y ago
  As someone with almost 40 years of Lisp experience, I firmly believe that the only correct place to call 'eval' is in the implementation of a REPL. For any other purpose that you might be tempted to use it for, there is a better way, that is not only more correct but also faster. Usually, the better way is to pass a function to be called rather than an expression to be evaluated. There are more legitimate uses of 'exec', of course, but one still needs to be very careful with it. If there really are Node modules that pass their input to 'exec', that strikes me as very poor design.
  - kazinator 9y ago
    > the only correct place to call 'eval' is in the implementation of a REPL. Don't forget also: if you're reading uncompiled code from a file, that's an occasion for eval.
    - ScottBurson 9y ago
      There are a handful of other valid uses, but there are many invalid ones. I recall, a couple of years ago, hearing about a Rails vulnerability caused by some API method calling 'eval' on one of its arguments if it happened to be a string, just as a convenience for some use case. The fact that it did so wasn't even documented. I was left with the impression that most programmers don't understand how to use 'eval', or rather, how and why not to use it.
paulclark 9y ago
"Vulnerable" due to eval() or exec()? Not really an "injection API".
weego 9y ago
tl;dr people pushing security tools for nodejs say security tools are the new most important aspect of node servers because some of them use calls that could maybe possibly be used in an unsafe manner but currently don't appear to be.
medleybron 9y ago
That's click-bait
deleted 9y ago
[deleted]
infogulch 9y ago
> thousands of modules may be vulnerable (emphasis mine) Apparently any module that uses eval or exec counts in "may", regardless of whether there's an actual vulnerability. Then using dependency analysis they extrapolate the 3% of packages that actually use these features to 20% that depend on something that use them. Maybe uses of eval or exec should be more closely audited for safety, but blanket stating that any use of core language features makes you vulnerable is just vacuous clickbait. This is a Synode sales pitch.
dspillett 9y ago
The title here (20% of all Node.js modules found vunerable to injection attacks) seems more sensationalised then that of the actual article: > Understanding and automatically preventing injection attacks on Node.js The article further guards the hyperbole with phrasing like "may be vulnerable" which is not reflected in the HN submission title. As click-baity as the title here could be interpreted, the article is highlighting what could be a significant problem: people often don't perform input validation well or create designs which make it difficult to do well at all. The article may be understating the issue by concentrating on specific types of attack: it seems to be referring only to calls to exec() and eval(), so could underestimating the problem by only considering served-side JS and shell/OS injection vectors. I would think that database vectors and client-side vectors (resulting on possible XSS attacks) are pretty common too, probably more so. They certainly are in a lot of code I've seen both online and in private codebases, due to lax input sanity checking or worse: architectures that make good validation in this respect practically impossible. This is not at all unique to node. > A staggering 90% of the call sites do not use any mitigation technique at all. Many libraries simply assume that the caller are performing input validation and only sending them sanitised commands. This is not safe, particularly in a publicly available library, as a lot of naive programmers will assume that the library is performing some validation and will raise exceptions in the presence of something dangerous. Even documenting the lack of validation won't help in many cases because how many people read documentation in detail until something goes wrong?! > Another 9% attempt to sanitise input using regular expressions. Unfortunately, most of those were not correctly implemented. This is a common problem too, the most problematic issue being people using regular expressions to try identify bad inputs. Trying to enumerate the bad is generally impossible as the range of bad inputs is usually infinitely larger then the range of good ones.
crispyporkbites 9y ago
This was posted 9 hours ago with the paper title as the name, and got 3 upvotes: https://news.ycombinator.com/item?id=16566587 https://news.ycombinator.com/item?id=16566587 I thought it was interesting and relevant for this audience, so I put a more sensational (but arguably accurate - you can argue the semantics of vulnerable all day) headline in the title to grab HN reader's attention. Now it is top of the front page with 40 votes in 25 minutes. If you want to read papers like this everyday, I got it from the morning paper mailing list: https://blog.acolyer.org/ https://blog.acolyer.org/
- dspillett 9y ago
  So basically: click-bait / vote-bait works, even with audiences that I expect to have a larger degree of technical literacy overall than the general populace. Sometimes I worry about humanity!
  - msla 9y ago
    Upvote is commonly used to mean "read it later" by this audience. It is by no means a vote on quality.
    - crispyporkbites 9y ago
      Is it? How do you see your upvoted content to read later?
      - grzm 9y ago
        In your profile there's a link to upvoted submissions.
    - minimaxir 9y ago
      This is an incorrect use of absolutes.
    - dvlsg 9y ago
      I sometimes upvote even when I disagree, if I think it will lead to quality discussion. I'm not sure this is one of those times, though.
js2 9y ago
Title should be changed to "Synode: understanding and automatically preventing injection attacks on Node.js" The issue is an important one to bring attention to, but as I leave this comment all the other comments are arguing whether the submitted title is accurate/click-bait.
larrykwg 9y ago
The research paper was accepted at NDSS, so thats something. Here is the video of the conference: https://www.youtube.com/watch?v=xVfLW2JhBq8 https://www.youtube.com/watch?v=xVfLW2JhBq8 IMHO static code analysis for this seems utterly useless, dead-end path to improve real-world NodeJS security. But this is definitely not a security software company trying to spread FUD like some suspecting here

"Vulnerable to injection attacks" seems like a bit of a stretch to me. Direct quote: "then about 20% of all modules turn out to directly or indirectly depend on at least one injection API" Where "injection API" is a reference to either the eval() or exec() functions.

shawabawa3 9y ago
To be fair they follow up with > A staggering 90% of the call sites do not use any mitigation technique at all. > Another 9% attempt to sanitise input using regular expressions. Unfortunately, most of those were not correctly implemented

bri3d 9y ago

This headline is entirely false - 20% of Node modules use 'eval' or 'exec'. While these are certainly less secure and a strong secure coding standard would probably ban them or at least reduce their use, it's entirely possible to use both in safe ways.

tonyarkles 9y ago
Further on: > 18,924 of all 51,627 call sites are found to be statically safe (36.66%) > The templates for the vast majority of call sites have at most one hole, and very few templates contain more than five. If you've got a dependency that calls eval or exec, those aren't great odds that they're doing it safely.
- acdha 9y ago
  That still requires you to know whether either is being called with user-provided values before you can say it's unsafe. If, for example, someone had a utility which resized user-uploaded images you couldn't say simply calling exec to run something like ImageMagick was unsafe before checking whether it used the user's filename.
  - tonyarkles 9y ago
    I think that's what they're saying in those quoted lines though. That 36% appeared to be statically-safe (i.e. the string sent to eval or exec was generated using only strings defined in the code) and the others were not.
    - acdha 9y ago
      True — it wasn't clear to me whether e.g. a random temporary filename would be considered static or only actual fixed strings in the code. (Similar questions would arise if e.g. a web app saved things using a database primary key so it's technically not static but the attacker doesn't control it even if they control the file contents)
      - tonyarkles 9y ago
        Ahhhhh nice, I hadn't considered the "random temporary filename" or "db primary key" aspect either. That's something that gives me a small chill, but it's not the end of the world compared to "arbitrary user input piped to exec()" Who knows how elaborate their static analyzer actually is in practice? I wrote a Python analyzer in grad school, and the results were both pretty interesting (type inference tracing through function calls) and pretty mediocre. It was also fiddly-as-hell to get working. Plus... the first rule of statically-analyzing dynamic languages is that the results get weird after the first eval()--anything could happen!
        acdha 9y ago
        Yeah, as a developer I see plenty of appeals to dynamic languages but they’re certainly a nightmare for tool authors. I know OpenStack was working on a security scanner for Python – I wonder what percentage of the open issues start with “this is probably very hard but…”
ynniv 9y ago
Possible, but unlikely. eval is particularly difficult to use correctly, and can almost always be replaced with something better.
Yaggo 9y ago
That 20% includes dependencies. I'm sure the majority of those are cli tools (webpack etc), i.e. they don't take unsanitized input from web forms etc.
weej 9y ago
Key word is vulnerable. There is a gap in protection and they are at risk to being potentially exploited.
- weej 9y ago
  Doesn't mean it isn't click bait. ;-)
Ajedi32 9y ago
It's not even that. It's more like "20% of Node modules use _or include a module somewhere in their dependency chain which uses_ 'eval' or 'exec'". So if my backup utility happens to depend on a wrapper library for node's Child Process module (which probably doesn't sanitize its inputs, as that would break quite a bit of functionality), it's considered "vulnerable".
- inetknght 9y ago
  And why shouldn't it be?
  - Ajedi32 9y ago
    Maybe because it's not sending user input into the "vulnerable" API? Or because it's not even calling the "vulnerable" API in the first place? Or because it's a system utility which will never be exposed to untrusted user input?
ScottBurson 9y ago
As someone with almost 40 years of Lisp experience, I firmly believe that the only correct place to call 'eval' is in the implementation of a REPL. For any other purpose that you might be tempted to use it for, there is a better way, that is not only more correct but also faster. Usually, the better way is to pass a function to be called rather than an expression to be evaluated. There are more legitimate uses of 'exec', of course, but one still needs to be very careful with it. If there really are Node modules that pass their input to 'exec', that strikes me as very poor design.
- kazinator 9y ago
  > the only correct place to call 'eval' is in the implementation of a REPL. Don't forget also: if you're reading uncompiled code from a file, that's an occasion for eval.
  - ScottBurson 9y ago
    There are a handful of other valid uses, but there are many invalid ones. I recall, a couple of years ago, hearing about a Rails vulnerability caused by some API method calling 'eval' on one of its arguments if it happened to be a string, just as a convenience for some use case. The fact that it did so wasn't even documented. I was left with the impression that most programmers don't understand how to use 'eval', or rather, how and why not to use it.

paulclark 9y ago

"Vulnerable" due to eval() or exec()? Not really an "injection API".

weego 9y ago

tl;dr people pushing security tools for nodejs say security tools are the new most important aspect of node servers because some of them use calls that could maybe possibly be used in an unsafe manner but currently don't appear to be.

medleybron 9y ago

That's click-bait

deleted 9y ago

[deleted]

infogulch 9y ago

> thousands of modules may be vulnerable (emphasis mine) Apparently any module that uses eval or exec counts in "may", regardless of whether there's an actual vulnerability. Then using dependency analysis they extrapolate the 3% of packages that actually use these features to 20% that depend on something that use them. Maybe uses of eval or exec should be more closely audited for safety, but blanket stating that any use of core language features makes you vulnerable is just vacuous clickbait. This is a Synode sales pitch.

dspillett 9y ago

The title here (20% of all Node.js modules found vunerable to injection attacks) seems more sensationalised then that of the actual article: > Understanding and automatically preventing injection attacks on Node.js The article further guards the hyperbole with phrasing like "may be vulnerable" which is not reflected in the HN submission title. As click-baity as the title here could be interpreted, the article is highlighting what could be a significant problem: people often don't perform input validation well or create designs which make it difficult to do well at all. The article may be understating the issue by concentrating on specific types of attack: it seems to be referring only to calls to exec() and eval(), so could underestimating the problem by only considering served-side JS and shell/OS injection vectors. I would think that database vectors and client-side vectors (resulting on possible XSS attacks) are pretty common too, probably more so. They certainly are in a lot of code I've seen both online and in private codebases, due to lax input sanity checking or worse: architectures that make good validation in this respect practically impossible. This is not at all unique to node. > A staggering 90% of the call sites do not use any mitigation technique at all. Many libraries simply assume that the caller are performing input validation and only sending them sanitised commands. This is not safe, particularly in a publicly available library, as a lot of naive programmers will assume that the library is performing some validation and will raise exceptions in the presence of something dangerous. Even documenting the lack of validation won't help in many cases because how many people read documentation in detail until something goes wrong?! > Another 9% attempt to sanitise input using regular expressions. Unfortunately, most of those were not correctly implemented. This is a common problem too, the most problematic issue being people using regular expressions to try identify bad inputs. Trying to enumerate the bad is generally impossible as the range of bad inputs is usually infinitely larger then the range of good ones.

crispyporkbites 9y ago

This was posted 9 hours ago with the paper title as the name, and got 3 upvotes: https://news.ycombinator.com/item?id=16566587 https://news.ycombinator.com/item?id=16566587 I thought it was interesting and relevant for this audience, so I put a more sensational (but arguably accurate - you can argue the semantics of vulnerable all day) headline in the title to grab HN reader's attention. Now it is top of the front page with 40 votes in 25 minutes. If you want to read papers like this everyday, I got it from the morning paper mailing list: https://blog.acolyer.org/ https://blog.acolyer.org/

dspillett 9y ago
So basically: click-bait / vote-bait works, even with audiences that I expect to have a larger degree of technical literacy overall than the general populace. Sometimes I worry about humanity!
- msla 9y ago
  Upvote is commonly used to mean "read it later" by this audience. It is by no means a vote on quality.
  - crispyporkbites 9y ago
    Is it? How do you see your upvoted content to read later?
    - grzm 9y ago
      In your profile there's a link to upvoted submissions.
  - minimaxir 9y ago
    This is an incorrect use of absolutes.
  - dvlsg 9y ago
    I sometimes upvote even when I disagree, if I think it will lead to quality discussion. I'm not sure this is one of those times, though.

js2 9y ago

Title should be changed to "Synode: understanding and automatically preventing injection attacks on Node.js" The issue is an important one to bring attention to, but as I leave this comment all the other comments are arguing whether the submitted title is accurate/click-bait.

larrykwg 9y ago

The research paper was accepted at NDSS, so thats something. Here is the video of the conference: https://www.youtube.com/watch?v=xVfLW2JhBq8 https://www.youtube.com/watch?v=xVfLW2JhBq8 IMHO static code analysis for this seems utterly useless, dead-end path to improve real-world NodeJS security. But this is definitely not a security software company trying to spread FUD like some suspecting here