Hacker News Reader

Comment by bri3d | Hacker News Reader

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.

View on HN

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.

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.

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.

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)

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!

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…”

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.

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.

> 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.

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.

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".

And why shouldn't it be?

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?

Possible, but unlikely. eval is particularly difficult to use correctly, and can almost always be replaced with something better.

Key word is vulnerable. There is a gap in protection and they are at risk to being potentially exploited.

Doesn't mean it isn't click bait. ;-)

Reply on news.ycombinator.com