0

I have the following two options to run a script with a string:

Option A:

var script = document.createElement('script');
script.innerHTML = "function foo(){console.log('Hello!')}";
script.type = "text/javascript";
document.body.appendChild(script);
foo();

Option B:

eval("function foo(){console.log('Hello!')}");
foo();

They both print Hello in the console.

If I want to let users dynamically run scripts in my app, which one is a better(safer, faster) option?

Zack Lee
  • 2,784
  • 6
  • 35
  • 77
  • 1
    I don't really think there is a *safer* option, personally i would use `eval()` as it require less lines of code. More about `eval` here https://stackoverflow.com/questions/197769/when-is-javascripts-eval-not-evil – Greedo Jul 07 '20 at 07:17
  • 1
    whichever you prefer - they are both script injections and they are both equally safe\dangerous – Bekim Bacaj Jul 07 '20 at 07:18
  • You should first define your security concerns. What do you mean by safe? I may be wrong but I think you should not concern yourself with eval vs script. More importantly, you can achieve some level of security using [CSP](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) and [CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS). – HosseinAgha Jul 07 '20 at 07:18

1 Answers1

1

If I want to let users dynamically run scripts in my app ...

There's nothing safe with this requirement, and none of the options is CSP friendly.

The "best" approach you could use though is Function, 'cause at least that doesn't reaches/target the local scope, like eval would do, keeping the execution within its function body scope.

The question now would be how to make the code available to the user without interfering with the rest of the code, or the previous call, or the global scope.

For this specific case I suggest a CommonJS like approach, so that your users must define what they are exporting in their code.

Example

const CommonJS = code => {
  const exports = {};
  const module = {exports};
  const runtime = Function(
    'module', 'exports',
    `"use strict";${code};return module`
  );
  return runtime(module, exports);
};

const {exports} = CommonJS(`
  module.exports = function foo(){console.log('Hello!')}
`);

exports();

Using this approach your users can define a single, or multiple, exports, like in Node.js, becoming familiar with the CommonJS module system, and avoiding any sort of global scope pollution, thanks to the "use strict" directive and the Function closure itself.

Alternatively, you can const foo = Function('return(' + code + ')')() but that requires code to be a single expression, so I would personally stick with the previous approach.

Andrea Giammarchi
  • 3,038
  • 15
  • 25
  • Thank you for your answer. How can I run `function foo(){console.log('Hello!')} function boo(){console.log('Meow!')}` and selectively call `foo()` or `boo()`? – Zack Lee Jul 07 '20 at 07:49
  • `exports.foo = function foo() {}; exports.boo = function boo(){};` at that point you `const {foo, boo} = CommonJS(code).exports;` and you can invoke either function. With a bit of analysis you could loop over `exports` key and if it's just one, and it's a function, you invoke it right away, or adjust the logic accordingly with your needs. – Andrea Giammarchi Jul 07 '20 at 07:53
  • I would really appreciate it if you could also add the full code that calls `foo()` and `boo()` as an `example2` in your post. – Zack Lee Jul 07 '20 at 08:18
  • 1
    code: `function foo(){console.log('Hello!')} function boo(){console.log('Meow!')} module.exports = {foo, boo};` run it: `const {foo, boo} = CommonJS(code).exports; foo(); boo();` – Andrea Giammarchi Jul 07 '20 at 09:45