Using setValueToScope and .nativeToPseudo with external libraries -- is this OK?

[mercmobily]

I cannot thank you enough for this interpreter.

I do have a question. In #143 you mentioned setValueToScope() which really solved my problem. I am writing a function like this:

const sharedCodeDbString = 'exports.hello2 = function(a) { return "Hello from shared DB " + a + " " + typeof moment }'

const runSharedCodeDb = function (funcName, ...args) {
  const fullCode = `exports = {}; ${sharedCodeDbString}; exports.${funcName}.apply(this, args);`
  const myInterpreter = new global.JSInterpreter.Interpreter(fullCode)
  myInterpreter.setValueToScope('args', myInterpreter.nativeToPseudo(args))
  myInterpreter.setValueToScope('moment', myInterpreter.nativeToPseudo(require('moment')))
  myInterpreter.setValueToScope('ejs', myInterpreter.nativeToPseudo(require('ejs')))
  myInterpreter.run()
  return myInterpreter.pseudoToNative(myInterpreter.value)
}

As I need the interpreted code to access moment and EJS. There is no documentation for setNativeToScope but it really is saving my butt :D

Am I doing something absolute obscene here? Also, if I let users define what's being interpreted, am I opening up my server to possible security flaws? (E.g. if a user knows of a vulnerability in moment, they can make the right call...).

THANK YOU again!

[NeilFraser]

Note that while nativeToPseudo can handle data structures (primitives, arrays, object maps, arrays, regexes, dates), it can also accept functions. Thus your users could be injecting wrapped function into the interpreter. However, it doesn't look like there's any ability to call those injected functions, so you should be fine.

If you really care about safety and want to be sure, create something like this:

function safePseudoToNative(interpreter, value) {
  if (typeof nativeObj === 'function') {
    throw Error('value is a function');
  }
  return interpreter.pseudoToNative(value);
}

Hmm, @cpcallen can I get your eye on this. This line: https://github.com/NeilFraser/JS-Interpreter/blob/master/interpreter.js#L2135 detects functions with instanceof not typeof. It seems that some objects can be not instanceof Function while being executable:

function foo() {alert('run')};
foo instanceof Function;  // true
typeof foo;  // "function"
Object.setPrototypeOf(foo, {});
foo instanceof Function;  // false
typeof foo;  // "function"

Looks like that line needs to be changed to typeof? There's no security issue here, worst case is that a non-executible function is wrapped (resulting in throwing) or an executable object is mapped (resulting in non-executability). I also wonder about the two instanceof checks above, for Date and RegExp.

[mercmobily]

Quick one... Here:

if (typeof nativeObj === 'function') {

Did you mean to wrote:

  if (typeof value === 'function') {

...?

[NeilFraser]

Yes, my mistake.

[cpcallen]

It seems that some objects can be not instanceof Function while being executable…

Yes, that is certainly true.

Looks like that line needs to be changed to typeof? There's no security issue here, worst case is that a non-executible function is wrapped (resulting in throwing) or an executable object is mapped (resulting in non-executability). I also wonder about the two instanceof checks above, for Date and RegExp.

Aside from the minor pedantry that "non-executable function" is arguably an oxymoron—if it's not executable (or at least constructible) it's not a function—the answer is maybe.

It turns out although we can detect function objects with typeof value === 'function' and arrays with Array.isArray(value), and there are a few other similar but less obvious tests for some other types, there is not in general a way to reliably detect the intrinsic type of an object nondestructively.

Aside 1: it used to be possible to do this: in ES5.1 you could use Object.prototype.toString.apply(value)—it would return [object Date] iff the object was a Date object, for example—but in ES6 you can defeat this test by setting value[Symbol.toStringTag] to whatever type you want. (This is probably at least in part by design, since ES6 added Proxies. It would be nice if they were handled a little more like POSIX handles symlinks, where they are treated transparently by default but you can make an explicit check if you want.)

Aside 2: TIL that node.js v10 added util.types, which provides methods like .isDate, .isRegExp, .isMapIterator and so on. Super useful if available, especially as util.types.isProxy might be the only reliable way to detect Proxy objects.

So it kind of comes down to a philosophical question.

1. If you basically trust the user to be competent and provide objects with the expected prototype, then instanceof is a good, clear check.

2. If you assume the users might be incompetent but not devious, check type by value[Symbol.toStringType (via Object.prototype.toString, if you need ES5 compatibility).

3. If the user might be malicious and you're on node.js, then check type by using util.types.

4. If the supplier of value might be malicious and you're not on node.js, use individually hand-crafted tests like seeing if Date.prototype.toString.call(value) or Map.prototype.get.call(value, undefined) throw.

Note that 4. is ugly and not fully general, since some types cannot be detected non-destructively—e.g., calling Object.getPrototypeOf([].values()).next.call(value) will throw iff value is not an Array Iterator Object, but if value is an array iterator it will get iterated. :-(