Cannot evel multiple lines of code

[kumavis]

eval('1+1; 2+2;')
//=> 4
evel('1+1; 2+2;')
//=> SyntaxError: Unexpected token ;

https://github.com/natevw/evel/blob/gh-pages/evel.js#L3

[natevw]

Hmm, tempting to fix that but please be aware that this project does not "work" as I'd originally hoped: #11
Sorry if there was confusion…I'll update the README right now to hopefully make this more clear :-/

[kumavis]

No problem! It was a noble attempt, has yielded insight and identified pitfalls. I'm hunting for a solution but it sounds like it will require a 'metacircular interpreter' approach where the code is actually interpreted by an interpreter built in js.

Hmm I do have one more idea to salvage this attempt though... let me try it out. As long as you can see this issue being trivially resolved.

[kumavis]

I have some questions - I'm kumavis on freenode irc - hit me up if you get a chance

[kumavis]

This is now the most critical issue, until I find another glaring security hole : )

[natevw]

I dealt with some annoying eval-like stuff in https://github.com/natevw/ddoc/blob/master/index.js#L19 — the string munging solution there is pretty lame and I'm not sure if it's relevant since it looks like I simply use the built-in "eval" to handle the multiple statements case.

Anyway, maybe I shouldn't worry — at the rate you're going you'll probably have figured this one out too by the time I'm up again 👍

[kumavis]

Yeah this is a tricky one eh. AFAICT, it would require modifying the code to have a return statement on the last line of the program. This would be easy using esprima and escodegen. Trying to think of a better hack.

[natevw]

Yeah, this seems tricky. Replacing semicolons with commas would work in some of the cases, but to do it right you'd still need to parse and so at that point you might as well just add the return statement.

One early thought, haven't fully reviewed where we're at with your recent progress, but IIRC originally the builtin eval was safely sandboxed by the previous wrapper iteration. The only "hole" was this (from MDN):

Functions created with the Function constructor do not create closures to their creation contexts; they always are created in the global scope. When running them, they will only be able to access their own local variables and global ones, not the ones from the scope in which the Function constructor was called. This is different from using eval with code for a function expression.

So (again haven't reviewed) maybe the way to handle eval is to a) not override _gObj.eval with evel and b) implement evel's "eval" using a evel.Function-wrapped function that calls (native) eval?

[kumavis]

Ah, that is an important discrepancy I was not aware of. and I think we may not be able to provide that 'feature' or eval.

var x = 123
evel('console.log(x)')
//=> undefined
eval('console.log(x)')
//=> 123
Function('console.log(x)')()
//=> 123 (wait wut, i thought it wasn't in scope)

EDIT: now i realize that var x = 123, when run as 'top-level code', implicitly becomes part of the global scope. Always wrap your code in closures! ( or use a module build system that does this for you, like commonjs and browserify )

[kumavis]

I mean if evel is "safe" then I think we wouldn't want it to be able to pull vars out of its surrounding environment.

[natevw]

To reiterate the discussion on #14, the unfettered use of eval is NOT okay — it's just as dangerous as the built-in Function. The reason is calling built-in eval through a variable breaks out of strict mode!

So this is looking like a tougher one. Note that we already don't support all the code that built-in Function/eval due (since not all existing code works under strict mode). So if we can't support this "easily", it might need to simply be another caveat.

[kumavis]

heh, looking at this issue again, this is a prrety significant 'caveat'

[natevw]

The discussion here got interspersed with various discussion of unpatched bypasses, so I thought I'd summarize where this ticket is at:

No one has proposed an correct-but-tiny way of doing this yet. I haven't ruled it out (hence leaving this ticket open) but I haven't stumbled across one yet either…

You can sometimes get away with using some relatively simple regex/string manipulation to e.g. wrap the code in a called function, adding a return in front of the last line/statement. However, I have concerns with this approach in fully general use. If you like this library's crazy-but-tiny experiment with sandboxing, but still need multi-line eval I would recommend wrapping evel with a preprocessor that does this, perhaps building on @duralog's proposal here — just be aware that you may be introducing edge cases where the return statement gets inserted into a comment or whatnot instead of where you meant it.

The only other way I [currently] know to do this right involves static analysis — truly parsing the JS code into a sort of "abstract syntax tree" and then manipulating that so (converted back to code) it will execute having eval's multiline semantics. I consider that level of JS-awareness to be far beyond the scope of evel, which is merely a clever combination of runtime tricks which still rely on the browser's JS engine to handle everything at the syntax level. AFAICT nearly all of the alternatives we've been collecting over on #8 are some variation or another of full-blown static analysis and/or beyond!

All this to say — I'm content to leave this open for a while yet, hoping someone will come along as happened with past vulnerabilities that seemed even more insurmountable. But meanwhile my advice is either:

• wrap evel in some of your own RegEx trick(s) that will work for the subset of inputs you expect
• bundle a big library maintained by compiler gurus and use that instead (and/or send untrusted code to "a separate process" serverside)

[Wilt]

You can easily wrap your code in a self executing function like this:

evel('(function(){1+1; 2+2;})()');

Maybe @natevw can implement the self executing function somehow native into the code?

[kumavis]

i think the requirement to return a value so it can be displayed is overvalued against just supporting multiple lines. you can always open the console and explicitly console.log() if you want to test a value.

[natevw]

Really old thread, but after reviewing this I'm still inclined to leave this limitation in place unless a reliable but simple trick is found.

I think these workarounds are acceptable in the meantime:

• need return value? → use directly
• need multiline? → wrap with "(function(){"+code+"})()" before passing in
• need return value of multiline code? → sorry, rewrite code so it will return from an IIFE similar to previous workaround