Instrumentation does not correctly distinguish direct and indirect eval in some obscure cases

[overlookmotel]

In both these cases, Babel plugin will incorrectly interpret eval() as indirect, as eval where it's used is a reference to a local variable.

const x = 1;
module.exports = ( (eval) => {
  return () => eval('x');
} )(eval);

const x = 123;
const e = eval;
{
  const eval = e;
  module.exports = () => eval('x');
}

NB Both cases are only possible in sloppy mode. It is not legal in strict mode to define a variable called eval.

More background here: http://perfectionkills.com/global-eval-what-are-the-options/

Only way to get it correct in these cases is to check if eval var is global eval at time of execution.

// Babel-transformed
const [ livepack_eval, livepack_preval ] = require('livepack/lib/init/eval.js');
const livepack_scopeId_1 = livepack_tracker();
const x = 1;
module.exports = ( (eval) => {
  return () => eval === livepack_eval
    ? eval(livepack_preval('x', [[1, livepack_scopeId_1,, ["x", "module", "exports"]]]))
    : eval('x');
} )( livepack_eval );

Could also be condensed to:

const [ livepack_eval, livepack_evalExec ] = require('livepack/lib/init/eval.js');
const livepack_scopeId_1 = livepack_tracker();
const x = 1;
module.exports = ( (eval) => {
  return () => livepack_evalExec(
    eval,
    (str, eval) => eval(str), // In strict mode functions, 2nd param would be omitted
    ['x'],
    [[1, livepack_scopeId_1,, ["x", "module", "exports"]]]
  );
} )( livepack_eval );

livepack_evalExec implementation:

const globalEval = eval;
function livepack_evalExec(localEval, exec, args, scopes) {
  // If `eval` where it's used is not global eval, execute it as a normal function
  if (localEval !== livepack_eval && localEval !== globalEval) return localEval(...args);

  // It's direct eval - process the evaluated expression before executing `eval()`
  const str = livepack_preval( args[0], scopes );
  return exec( str, globalEval );
}

Low priority. These cases are pretty obscure.

[overlookmotel]

Tricky case where a local eval var is defined in a sloppy-mode function, but called in a strict mode function:

const x = 123;
module.exports = (eval => (
  () => {
    'use strict';
    return eval(x);
  }
))(eval);

In this case, substituting eval() for livepack_evalExec (as defined above) would not work.

[overlookmotel]

Here's an implementation which solves both this and #51:

// lib/init/eval.js
const nativeEval = eval;
const evalShim = {
  eval(str) {
    return nativeEval(preval(str));
  }
}.eval;

function shimGlobal() {
  global.eval = evalShim;
}
shimGlobal();

function preval(str, scopes, filename) {
  return str; // Actually would do babel-transform on string
}

module.exports = {
  shim(val) {
    return val === nativeEval ? evalShim : val;
  },
  unshim(val) {
    return val === evalShim ? nativeEval : val;
  },
  shimGlobal,
  exec(localEval, exec, args, scopes, filename) {
    // If `eval` where it's used is not global eval, execute it as a normal function
    const isShimmedEval = localEval === evalShim;
    if ( !isShimmedEval && localEval !== nativeEval ) return localEval(...args);

    // It's direct eval - process the evaluated expression before executing `eval()`
    let str = preval( args[0], scopes, filename );
    if (isShimmedEval) {
      // `eval` where it's being executed refers to global, which is shimmed.
      // Temporarily restore `global.eval`.
      global.eval = nativeEval;
      str = `livepack_eval.shimGlobal();${str}`;
    }

    try {
      return exec(str);
    } catch (err) {
      // Fallback in case eval fails with syntax error and `livepack_eval.shimGlobal()`
      // inside eval expression is not executed
      if (isShimmedEval) shimGlobal();
      throw err;
    }
  }
};

Then this input:

// src/index.js
const Object = 123;
const fn1 = ( eval => () => {
  'use strict';
  const indirectEval = eval;
  return [
    eval('Object'),
    indirectEval('Object')
  ];
} )(eval);

const fn2 = (() => {
  const { eval } = { eval: global.eval };
  return () => {
    'use strict';
    return [
      eval('Object'),
      (0, eval)('Object')
    ];
  };
})();

const fn3 = () => [
  eval('Object'),
  (0, eval)('Object')
];

module.exports = [ fn1, fn2, fn3 ];

is transformed by Babel plugin to:

const livepack_eval = require('/path/to/app/node_modules/livepack/lib/init/eval.js'),
  livepack_filename = '/path/to/app/src/index.js';
const livepack_scopeId_1 = 1;
const Object = 123;
const fn1 = ( (eval) => {
  eval = livepack_eval.unshim(eval);
  return () => {
    'use strict';
    const indirectEval = livepack_eval.shim(eval);
    return [
      livepack_eval.exec(
        eval,
        str => eval(str),
        ['Object'],
        [ [ 1, livepack_scopeId_1, , [ 'Object', 'module', 'exports' ] ] ],
        livepack_filename
      ),
      indirectEval('Object')
    ];
  };
} )(eval);

const fn2 = ( () => {
  const {eval: livepack_temp_1} = {eval: global.eval};
  const eval = livepack_eval.unshim(livepack_temp_1);
  return () => {
    'use strict';
    return [
      livepack_eval.exec(
        eval,
        str => eval(str),
        ['Object'],
        [ [ 1, livepack_scopeId_1, , [ 'Object', 'module', 'exports' ] ] ],
        livepack_filename
      ),
      ( 0, livepack_eval.shim(eval) )('Object')
    ];
  };
} )();

const fn3 = () => [
  livepack_eval.exec(
    eval,
    str => eval(str),
    ['Object'],
    [ [ 1, livepack_scopeId_1, , [ 'Object', 'module', 'exports' ] ] ],
    livepack_filename
  ),
  (0, eval)('Object')
];

module.exports = [ fn1, fn2, fn3 ];

Notes:

• global.eval is shimmed.
• eval() is replaced with livepack_eval.exec().
• If a var called eval is defined, its value is unshimmed with livepack_eval.unshim().
• eval referring to global scope is left untouched.
• eval referring to local var is re-shimmed with livepack_eval.shim().
• Temp var is necessary when unshimming if it's not a direct assigment.

Tricky part in function serialization is restoring the original object destructuring from const { eval: livepack_temp_1 } = ....

The above implementation does not deal with passing Livepack's internal vars into scope of eval-ed expression. The intent of passing filename into livepack_eval.exec() is so livepack_eval can be global instead of a separate instance being created for each module - which should simplify implementation versus how it is now.

[overlookmotel]

Prepending livepack_eval.shimGlobal(); on start of evaluated string in above implementation fails to take into account if the evaluated string begins with 'use strict'.

e.g. if evaluated string is '"use strict"; ( () => { NaN = 1; return 123; } )();'

Evaluating this string with eval() throws an error because arrow function is defined in strict mode and so NaN = 1 throws. If string has livepack_eval.shimGlobal(); prepended to it before evaluation, it no longer throws - the arrow function is no longer defined in strict mode.

It's not as simple as checking if string begins with 'use strict'; as could also have comments or whitespace before it.

Solution: Insert livepack_eval.shimGlobal(); during the Babel transform of the string (in preval()), after any 'use strict' directive.

NB Another tempting solution is to prepend livepack_eval.shimGlobal(); but wrap string in an IIFE i.e. 'use strict'; x => livepack_eval.shimGlobal(); ( () => { 'use strict'; return x; } )(). However, wrapping in an IIFE changes behavior of defining vars with var statement in sloppy mode, where it creates a var in outer scope. e.g. calling () => ( eval('var x = 123'), x ) returns 123. This is not the case for () => ( eval('(() => {var x = 123})()'), x ).

[overlookmotel]

Once this is implemented, enable the tests in:

livepack/test/strict.test.js

Line 823 in 55bf630

// These tests don't work at present, due to bug with how Livepack handles `eval`.

[overlookmotel]

The above would not work if user code prevents modification of global.eval with e.g. Object.freeze(global).

It could be shimmed with a getter/setter instead:

let globalEval = eval,
  shimmedEval;
Object.defineProperty(global, 'eval', {
  get() { return shimmedEval || globalEval; },
  set(v) { globalEval = v; }
});  

shimmedEval can be set or unset depending on whether value of eval should be shimmed or not at any given time.

[overlookmotel]

Solution in comment above can be simplified a bit:

Rather than shimming and unshimming local vars called eval, could just rename all uses of local vars called eval to livepack_localEval.

• eval would then never be blocked in a local scope and would always refer to global.eval (which is shimmed).
• Only direct eval() calls would be transformed to livepack_tracker.evalDirect().

The example above would be instrumented as:

const Object = 123;
const fn1 = ( livepack_localEval => () => {
  'use strict';
  const indirectEval = livepack_localEval;
  return [
    livepack_tracker.evalDirect(
      'Object', livepack_localEval, livepack_temp1 => eval(livepack_temp1), [ /* ... */ ]
    ),
    indirectEval('Object')
  ];
} )(eval);

const fn2 = (() => {
  const { eval: livepack_localEval } = { eval: global.eval };
  return () => {
    'use strict';
    return [
      livepack_tracker.evalDirect(
        'Object', livepack_localEval, livepack_temp2 => eval(livepack_temp2), [ /* ... */ ]
      ),
      (0, livepack_localEval)('Object')
    ];
  };
})();

const fn3 = () => [
  livepack_tracker.evalDirect(
    'Object', eval, livepack_temp3 => eval(livepack_temp3), [ /* ... */ ]
  ),
  (0, eval)('Object')
];

module.exports = [ fn1, fn2, fn3 ];

Handling references to local eval var in direct eval()

There is a complication with direct eval() calls where the internal vars prefix changes internally, as would have to ensure livepack1_localEval inside direct eval() stays in sync with external var livepack_localEval, which may not be a constant.

If in sloppy mode, could use with( o ) {} where o has a getter/setter for livepack1_localEval - it's legal to assign to eval in sloppy mode code.

In strict mode, no assignments to eval are possible, so could just replace eval with livepack1_getLocalEval() which returns value of livepack_localEval outside the eval() code.

[overlookmotel]

Here's a solution which also solves #51 and #457 (comment).

Direct eval(x) is replaced with:

(livepack_temp1 = livepack_tracker.evalDirect(eval, [x], [ /* scopes */ ]))[0]
  ? eval(livepack_temp1[1])
  : livepack_temp1[1]

Set-up code:

const nativeEval = eval;
const evalShim = {
  eval(code) {
    if (typeof code === 'string') code = instrumentCode(code);
    return nativeEval(code);
  }
}.eval;

let useNative = false;
Object.defineProperty( global, 'eval', {
  get() {
    if (!useNative) return evalShim;
    useNative = false;
    return nativeEval;
  }
} );

livepack_tracker.evalDirect = ( localEval, values, scopes ) => {
  if ( localEval !== evalShim || typeof values[0] !== 'string' ) {
    return [ false, localEval(...values) ];
  }
  useNative = true;
  return [ true, instrumentCode(values[0], scopes) ];
};

global.eval is turned into a getter. Usually it returns the shim of eval (which is called when indirect eval call. In case of a direct eval() call, evalDirect tells it to temporarily switch global.eval to the "real" native eval.

This also maintains the original order of operations - eval is looked up first, then the values in eval() call's arguments are evaluated.

The example above would be instrumented as:

const Object = 123;
const fn1 = ( livepack_evalLocal => () => {
  'use strict';
  let livepack_temp1, livepack_temp2;
  const indirectEval = livepack_evalLocal;
  return [
    (livepack_temp1 = livepack_tracker.evalDirect(
      livepack_evalLocal, ['Object'], [ /* scopes */ ]
    ))[0] ? eval(livepack_temp1[1]) : livepack_temp1[1]
    (livepack_temp2 = livepack_tracker.evalDirect(
      indirectEval, ['Object'], [ /* scopes */ ]
    ))[0] ? eval(livepack_temp2[1]) : livepack_temp2[1]
  ];
} )(eval);

const fn2 = (() => {
  const { eval: livepack_evalLocal } = { eval: global.eval };
  return () => {
    'use strict';
    let livepack_temp3;
    return [
      (livepack_temp3 = livepack_tracker.evalDirect(
        livepack_evalLocal, ['Object'], [ /* scopes */ ]
      ))[0] ? eval(livepack_temp3[1]) : livepack_temp3[1],
      (0, livepack_evalLocal)('Object')
    ];
  };
})();

const fn3 = () => {
  let livepack_temp4;
  return [
    (livepack_temp4 = livepack_tracker.evalDirect(
      eval, ['Object'], [ /* scopes */ ]
    ))[0] ? eval(livepack_temp4[1]) : livepack_temp4[1],
    (0, eval)('Object')
  ];
};

module.exports = [ fn1, fn2, fn3 ];

[overlookmotel]

Mostly implemented now on instrument-eval branch.

Remaining work:

• Handle function declarations called eval
• Tests

[overlookmotel]

Implementation on instrument-eval branch has become impractically convoluted.

global.eval is shimmed so that indirect eval calls e.g. const x = global.eval; x() is caught and the code is instrumented (good). The shim is deactivated when it's being used in an instumented direct eval() call, so that eval() uses the native eval (complicated but OK).

The difficulty is this can be interfered with by a local var in source called eval, which has the value of global.eval. This can't be "switched off" in the same way as the global shim.

const {eval} = global;
eval('foo');

The solution I've implemented is that all local vars called eval are renamed to livepack_localEval so they don't block access to the shimmed global.eval.

This produces a lot of complications, because function declarations called eval need to be renamed to livepack_localEval without their .name property being altered, internal vars prefix num change needs to be handled so livepack2_localEval is re-routed to livepack_localEval in outer scope etc.

#485 might be a better solution.

Or could try creating a setter for the local var so its value can be "switched" to native eval like the global shim.

That would be tricky where the local var is a const. Would need to either:

1. Convert const to let and turn any assignments to the vars in that var declaration into code which throws.
2. Use a with (...) {} statement to dynamically override the value when calling eval(). const {eval} = global, x = eval('foo'); would be difficult (where does the with() {} go?)