From 9ecdba3a22f9e55f0b57923de2d4fa6492b9c7ef Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 23 Dec 2023 02:06:03 +0000 Subject: [PATCH] Freeze vars in function containing `eval()` even if `eval` is local var [fix] --- lib/instrument/visitors/eval.js | 73 ++++++++++++--------------------- test/eval.test.js | 39 ++++++++++++++++++ 2 files changed, 66 insertions(+), 46 deletions(-) diff --git a/lib/instrument/visitors/eval.js b/lib/instrument/visitors/eval.js index eeb6a278..49867e8b 100644 --- a/lib/instrument/visitors/eval.js +++ b/lib/instrument/visitors/eval.js @@ -47,10 +47,14 @@ function visitEval(node, parent, key, state) { state.fileContainsFunctionsOrEval = true; // Queue instrumentation on 2nd pass - state.secondPass( - instrumentEval, node, isEvalCall, parent, key, state.currentBlock, fn, - state.isStrict, canUseSuper, state.currentSuperIsProto, state - ); + if (isEvalCall) { + state.secondPass( + instrumentEvalCall, + parent, state.currentBlock, fn, state.isStrict, canUseSuper, state.currentSuperIsProto, state + ); + } else { + state.secondPass(instrumentEvalIdentifier, node, state.currentBlock, parent, key, state); + } } /** @@ -76,34 +80,6 @@ function flagFunctionsAsContainingEval(fn) { } } -/** - * Instrument `eval` - either direct `eval()` or e.g. `x = eval;` / `(0, eval)('...')`. - * @param {Object} node - `eval` identifier AST node - * @param {boolean} isEvalCall - `true` if is eval call - * @param {Object|Array} parent - Parent AST node/container - * @param {string|number} key - Node's key on parent AST node/container - * @param {Object} block - Block object for block `eval` is in - * @param {Object} [fn] - Function object for function `eval` is in (`undefined` if not in a function) - * @param {boolean} isStrict - `true` if is strict mode - * @param {boolean} canUseSuper - `true` if `super()` can be used in `eval()` - * @param {boolean} superIsProto - `true` if `super` refers to class prototype - * @param {Object} state - State object - * @returns {undefined} - */ -function instrumentEval( - node, isEvalCall, parent, key, block, fn, isStrict, canUseSuper, superIsProto, state -) { - // Check `eval` is global - if (!isGlobalEval(block)) return; - - // Handle either `eval()` call or `eval` identifier - if (isEvalCall) { - instrumentEvalCall(parent, block, fn, isStrict, canUseSuper, superIsProto, state); - } else { - instrumentEvalIdentifier(node, parent, key, state); - } -} - /** * Instrument `eval()` call. * Convert `eval()` to `livepack_tracker.evalDirect()`. @@ -128,7 +104,8 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP // Capture all vars in scopes above const scopeNodes = [], varNamesUsed = new Set(); - let isExternalToFunction = false; + let isExternalToFunction = false, + evalIsLocal = false; do { // Check if block is external to function if (!isExternalToFunction && fn && block.id < fn.id) isExternalToFunction = true; @@ -159,6 +136,8 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP varNamesUsed.add(varName); + if (varName === 'eval') evalIsLocal = true; + // Create array for var `[varName, isConst, isSilentConst, argNames, tempVarValue]`. // NB: Assignment to var called `arguments` or `eval` is illegal in strict mode. // This is relevant as it affects whether var is flagged as mutable or not. @@ -200,6 +179,14 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP } } while (block = block.parent); // eslint-disable-line no-cond-assign + // If `eval` is not refering to global, don't replace `eval()` + // TODO: This isn't perfect, but problem is local `eval` var may a reference to global `eval`, + // but if so it will have been swapped for `livepack_tracker.evalIndirect`, and no way to reliably + // restore `eval` in local scope to native `eval` in order to call it if in strict mode. + // e.g. `const x = 1; (eval => { 'use strict'; eval('x'); })(eval);` + // https://github.com/overlookmotel/livepack/issues/485 would be a solution to this. + if (evalIsLocal) return; + // Replace `eval(x)` with // `livepack_tracker.evalDirect( // x, @@ -232,30 +219,24 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP * Instrument `eval` used standalone (not as an `eval()` call). * Replace `eval` with `livepack_tracker.evalIndirect`. * @param {Object} node - `eval` identifier AST node + * @param {Object} block - Block object for block `eval` is in * @param {Object|Array} parent - Parent AST node/container * @param {string|number} key - Node's key on parent AST node/container * @param {Object} state - State object * @returns {undefined} */ -function instrumentEvalIdentifier(node, parent, key, state) { +function instrumentEvalIdentifier(node, block, parent, key, state) { + // Don't alter if `eval` doesn't refer to global `eval` + do { + if (block.bindings.has('eval')) return; + } while (block = block.parent); // eslint-disable-line no-cond-assign + parent[key] = copyLocAndComments( t.memberExpression(state.trackerVarNode, t.identifier('evalIndirect')), node ); } -/** - * Determine if `eval` identifier refers to global `eval` (not var called `eval` defined in file). - * @param {Object} block - Block object that `eval` is in. - * @returns {boolean} - `true` if `eval` is a global var - */ -function isGlobalEval(block) { - do { - if (block.bindings.has('eval')) return false; - } while (block = block.parent); // eslint-disable-line no-cond-assign - return true; -} - /** * If `super` is usable in this `eval()`, activate `super` block. * @param {Object} [fn] - Function object diff --git a/test/eval.test.js b/test/eval.test.js index fb2e070b..eaec1b86 100644 --- a/test/eval.test.js +++ b/test/eval.test.js @@ -1695,6 +1695,45 @@ describe('eval', () => { }); }); + describe('freezes vars where `eval` is local var', () => { + itSerializes('external vars', { + in: ` + const {eval} = global, + extA = 1, extB = 2; + module.exports = function(module, exports) { + return eval('[extA, extB]'); + }; + `, + out: `(0,eval)(" + (eval,extA,extB)=>function(module,exports){return eval(\\"[extA, extB]\\")} + ")(eval,1,2)`, + validate(fn) { + expect(fn).toBeFunction(); + expect(fn()).toEqual([1, 2]); + } + }); + + itSerializes('internal vars', { + in: ` + module.exports = function(module, exports) { + const {eval} = global, + intA = 1, intB = 2; + return eval('[intA, intB]'); + }; + `, + out: `(0,eval)(" + (function(module,exports){ + const{eval}=global,intA=1,intB=2; + return eval(\\"[intA, intB]\\") + }) + ")`, + validate(fn) { + expect(fn).toBeFunction(); + expect(fn()).toEqual([1, 2]); + } + }); + }); + describe('does not freeze vars internal to function where not accessible to eval', () => { describe('in nested blocks', () => { itSerializes('where vars differently named from frozen vars', {