From 3848bca7941ed86d62e6a7108b960201613d2172 Mon Sep 17 00:00:00 2001 From: Timofey Kachalov Date: Tue, 27 Jan 2026 17:20:23 +0400 Subject: [PATCH] Backport some fixes from Pro version (#1374) --- CHANGELOG.md | 2 + src/analyzers/scope-analyzer/ScopeAnalyzer.ts | 114 ++++++++++++++++++ src/node/NodeUtils.ts | 5 + .../scope-analyzer/ScopeAnalyzer.spec.ts | 78 ++++++++++++ .../fixtures/annex-b-function-hoisting.js | 39 ++++++ .../fixtures/annex-b-let-const-shadowing.js | 9 ++ .../fixtures/annex-b-strict-mode.js | 11 ++ ...ObjectPatternPropertiesTransformer.spec.ts | 59 +++++++++ .../destructuring-default-outer-var.js | 8 ++ .../nested-destructuring-default-outer-var.js | 8 ++ .../simple-default-param-outer-var.js | 8 ++ test/helpers/evalLocal.ts | 9 ++ 12 files changed, 350 insertions(+) create mode 100644 test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-function-hoisting.js create mode 100644 test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-let-const-shadowing.js create mode 100644 test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-strict-mode.js create mode 100644 test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/destructuring-default-outer-var.js create mode 100644 test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/nested-destructuring-default-outer-var.js create mode 100644 test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/simple-default-param-outer-var.js create mode 100644 test/helpers/evalLocal.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c289cdda..8fbbaf6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ v5.2.0 --- * Skip obfuscation of `process.env.*` * Fixed `controlFlowFlattening` breaking short-circuit evaluation with spread operator and conditional objects. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1372 +* Fix Annex B function hoisting: block-scoped function declarations are now correctly linked to references outside the block in non-strict mode +* Fixed `NodeUtils.cloneRecursive` corrupting `range` property when cloning AST nodes, causing scope analysis to incorrectly resolve destructuring default parameter references v5.1.0 --- diff --git a/src/analyzers/scope-analyzer/ScopeAnalyzer.ts b/src/analyzers/scope-analyzer/ScopeAnalyzer.ts index 169e3339..b4001cf6 100644 --- a/src/analyzers/scope-analyzer/ScopeAnalyzer.ts +++ b/src/analyzers/scope-analyzer/ScopeAnalyzer.ts @@ -84,6 +84,11 @@ export class ScopeAnalyzer implements IScopeAnalyzer { sourceType: ScopeAnalyzer.sourceTypes[i] }); + // Fix Annex B function hoisting references + // eslint-scope doesn't implement Annex B semantics where function declarations + // in blocks also create a var-hoisted binding in the enclosing function scope + this.fixAnnexBFunctionHoisting(); + return; } catch (error) { if (i < sourceTypeLength - 1) { @@ -117,6 +122,101 @@ export class ScopeAnalyzer implements IScopeAnalyzer { return scope; } + /** + * Fix Annex B function hoisting references. + * + * In non-strict mode, function declarations in blocks have dual binding: + * 1. A block-scoped binding (handled by eslint-scope) + * 2. A var-hoisted binding in the enclosing function scope (NOT handled by eslint-scope) + * + * This method merges block-scoped function declarations into the enclosing + * function scope and links unresolved references. + */ + private fixAnnexBFunctionHoisting(): void { + if (!this.scopeManager) { + return; + } + + this.walkScopes(this.scopeManager.globalScope, (scope: eslintScope.Scope) => { + if (scope.type !== 'block' && scope.type !== 'switch') { + return; + } + + // Skip strict mode scopes - Annex B doesn't apply + if (scope.isStrict) { + return; + } + + const functionScope = scope.variableScope; + + if (!functionScope) { + return; + } + + for (let i = scope.variables.length - 1; i >= 0; i--) { + const variable = scope.variables[i]; + + const isFunctionDeclaration = variable.defs.some( + (def) => def.type === 'FunctionName' && def.node?.type === 'FunctionDeclaration' + ); + + if (!isFunctionDeclaration) { + continue; + } + + // Find existing variable with the same name in function scope (shadowing case) + const outerVariable = functionScope.variables.find((v) => v.name === variable.name && v !== variable); + + // Per Annex B.3.3, hoisting only applies if outer binding is var/function (not let/const) + const isOuterLetOrConst = outerVariable?.defs.some( + (def) => def.type === 'Variable' && (def.parent?.kind === 'let' || def.parent?.kind === 'const') + ); + + // Skip Annex B hoisting if there's a let/const with the same name + if (isOuterLetOrConst) { + continue; + } + + const targetVariable = outerVariable ?? variable; + + if (outerVariable) { + // Merge inner function's identifiers and references into outer + outerVariable.identifiers.push(...variable.identifiers); + outerVariable.references.push(...variable.references); + } else { + // Move variable to function scope so references can find it + functionScope.variables.push(variable); + } + + // Remove from block scope + scope.variables.splice(i, 1); + + // Link "through" references with matching name to the target variable + this.linkThroughReferences(variable.name, functionScope, targetVariable); + } + }); + } + + /** + * Link unresolved "through" references to a variable. + * + * @param {string} name - The variable name to match + * @param {Scope} scope - The scope to start searching from + * @param {Variable} targetVariable - The variable to link references to + */ + private linkThroughReferences(name: string, scope: eslintScope.Scope, targetVariable: eslintScope.Variable): void { + for (let i = scope.through.length - 1; i >= 0; i--) { + if (scope.through[i].identifier.name === name) { + targetVariable.references.push(scope.through[i]); + scope.through.splice(i, 1); + } + } + + for (const childScope of scope.childScopes) { + this.linkThroughReferences(name, childScope, targetVariable); + } + } + /** * @param {Scope} scope */ @@ -150,4 +250,18 @@ export class ScopeAnalyzer implements IScopeAnalyzer { this.sanitizeScopes(childScope); } } + + /** + * Walk through all scopes in the scope tree + * + * @param {Scope} scope - Starting scope + * @param {Function} callback - Function to call for each scope + */ + private walkScopes(scope: eslintScope.Scope, callback: (scope: eslintScope.Scope) => void): void { + callback(scope); + + for (const childScope of scope.childScopes) { + this.walkScopes(childScope, callback); + } + } } diff --git a/src/node/NodeUtils.ts b/src/node/NodeUtils.ts index 2834fb66..11cf6f30 100644 --- a/src/node/NodeUtils.ts +++ b/src/node/NodeUtils.ts @@ -122,6 +122,11 @@ export class NodeUtils { return node; } + // Handle primitives directly - don't try to clone them as objects + if (typeof node !== 'object') { + return node; + } + const copy: Partial = {}; const nodeKeys: (keyof T)[] = <(keyof T)[]>Object.keys(node); diff --git a/test/functional-tests/analyzers/scope-analyzer/ScopeAnalyzer.spec.ts b/test/functional-tests/analyzers/scope-analyzer/ScopeAnalyzer.spec.ts index b0f82d72..188f6d0c 100644 --- a/test/functional-tests/analyzers/scope-analyzer/ScopeAnalyzer.spec.ts +++ b/test/functional-tests/analyzers/scope-analyzer/ScopeAnalyzer.spec.ts @@ -2,6 +2,7 @@ import 'reflect-metadata'; import { assert } from 'chai'; +import { evalLocal } from '../../../helpers/evalLocal'; import { readFileAsString } from '../../../helpers/readFileAsString'; import { JavaScriptObfuscator } from '../../../../src/JavaScriptObfuscatorFacade'; @@ -43,5 +44,82 @@ describe('ScopeAnalyzer', () => { assert.equal(error, null); }); }); + + describe('Variant #2: Annex B function hoisting', () => { + describe('Variant #1: basic block-scoped function hoisting', () => { + const samplesCount: number = 50; + + let testFunc: () => void; + + beforeEach(() => { + const code: string = readFileAsString(__dirname + '/fixtures/annex-b-function-hoisting.js'); + + testFunc = () => { + for (let i = 0; i < samplesCount; i++) { + const obfuscatedCode: string = JavaScriptObfuscator.obfuscate(code, { + seed: i + }).getObfuscatedCode(); + + const result = evalLocal(obfuscatedCode); + + if (result.test1 !== 'foo') { + throw new Error('test1 failed: expected foo, got ' + result.test1); + } + + if (result.test2 !== 'bar') { + throw new Error('test2 failed: expected bar, got ' + result.test2); + } + } + }; + }); + + it('should correctly handle Annex B function hoisting references', () => { + assert.doesNotThrow(testFunc); + }); + }); + + describe('Variant #2: strict mode should not apply Annex B hoisting', () => { + let testFunc: () => void; + + beforeEach(() => { + const code: string = readFileAsString(__dirname + '/fixtures/annex-b-strict-mode.js'); + + testFunc = () => { + const obfuscatedCode: string = JavaScriptObfuscator.obfuscate(code, { + seed: 12345 + }).getObfuscatedCode(); + + eval(obfuscatedCode); + }; + }); + + it('should correctly handle strict mode block-scoped functions', () => { + assert.doesNotThrow(testFunc); + }); + }); + + describe('Variant #3: let/const shadowing should prevent Annex B hoisting', () => { + let testFunc: () => void; + + beforeEach(() => { + const code: string = readFileAsString(__dirname + '/fixtures/annex-b-let-const-shadowing.js'); + + testFunc = () => { + const obfuscatedCode: string = JavaScriptObfuscator.obfuscate(code, { + seed: 12345 + }).getObfuscatedCode(); + + const result = eval(obfuscatedCode); + if (result !== 'outer') { + throw new Error('Expected outer, got: ' + result); + } + }; + }); + + it('should not hoist when let/const shadows the function name', () => { + assert.doesNotThrow(testFunc); + }); + }); + }); }); }); diff --git a/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-function-hoisting.js b/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-function-hoisting.js new file mode 100644 index 00000000..0dc18de1 --- /dev/null +++ b/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-function-hoisting.js @@ -0,0 +1,39 @@ +// Basic Annex B case: function in block referenced after block +function test1() { + if (true) { + function foo() { + return 'foo'; + } + } + return foo(); +} + +// Function in switch case +function test2(x) { + switch (x) { + case 1: + function bar() { + return 'bar'; + } + break; + } + return bar(); +} + +// Multiple blocks with same function name +function test3() { + if (true) { + function baz() { + return 'first'; + } + } + if (false) { + function baz() { + return 'second'; + } + } + return baz(); +} + +// Return results for testing +({ test1: test1(), test2: test2(1) }); diff --git a/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-let-const-shadowing.js b/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-let-const-shadowing.js new file mode 100644 index 00000000..fee0550b --- /dev/null +++ b/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-let-const-shadowing.js @@ -0,0 +1,9 @@ +function test() { + let foo = 'outer'; + if (true) { + function foo() { return 'inner'; } + foo(); // block-scoped foo + } + return foo; // should be 'outer', not the function +} +test(); diff --git a/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-strict-mode.js b/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-strict-mode.js new file mode 100644 index 00000000..6bff144b --- /dev/null +++ b/test/functional-tests/analyzers/scope-analyzer/fixtures/annex-b-strict-mode.js @@ -0,0 +1,11 @@ +'use strict'; +function test() { + let foo; + if (true) { + function foo() { return 'inner'; } + foo(); // This refers to block-scoped foo + } + // foo here is the outer let, which is undefined + return typeof foo; +} +test(); diff --git a/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/ObjectPatternPropertiesTransformer.spec.ts b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/ObjectPatternPropertiesTransformer.spec.ts index 69a87036..dd43acb1 100644 --- a/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/ObjectPatternPropertiesTransformer.spec.ts +++ b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/ObjectPatternPropertiesTransformer.spec.ts @@ -3,6 +3,7 @@ import { assert } from 'chai'; import { NO_ADDITIONAL_NODES_PRESET } from '../../../../../src/options/presets/NoCustomNodes'; import { readFileAsString } from '../../../../helpers/readFileAsString'; +import { evalLocal } from '../../../../helpers/evalLocal'; import { JavaScriptObfuscator } from '../../../../../src/JavaScriptObfuscatorFacade'; @@ -142,4 +143,62 @@ describe('ObjectPatternPropertiesTransformer', () => { }); }); }); + + describe('Variant #3: destructuring default parameter with var shadowing', () => { + describe('Variant #1: destructuring default should reference outer var, not inner var', () => { + let code: string; + let obfuscatedCode: string; + + before(() => { + code = readFileAsString(__dirname + '/fixtures/destructuring-default-outer-var.js'); + + obfuscatedCode = JavaScriptObfuscator.obfuscate(code, { + ...NO_ADDITIONAL_NODES_PRESET + }).getObfuscatedCode(); + }); + + it('should correctly resolve destructuring default to outer variable', () => { + // Default parameter `a = x` should use outer 'x' value ('outer'), + // NOT the inner var x = 'inner' which is in the function body scope + assert.equal(evalLocal(code), 'outer'); + assert.equal(evalLocal(obfuscatedCode), evalLocal(code)); + }); + }); + + describe('Variant #2: nested destructuring default should reference outer var', () => { + let code: string; + let obfuscatedCode: string; + + before(() => { + code = readFileAsString(__dirname + '/fixtures/nested-destructuring-default-outer-var.js'); + + obfuscatedCode = JavaScriptObfuscator.obfuscate(code, { + ...NO_ADDITIONAL_NODES_PRESET + }).getObfuscatedCode(); + }); + + it('should correctly resolve nested destructuring default to outer variable', () => { + assert.equal(evalLocal(code), 'outer'); + assert.equal(evalLocal(obfuscatedCode), evalLocal(code)); + }); + }); + + describe('Variant #3: simple default param with var shadow', () => { + let code: string; + let obfuscatedCode: string; + + before(() => { + code = readFileAsString(__dirname + '/fixtures/simple-default-param-outer-var.js'); + + obfuscatedCode = JavaScriptObfuscator.obfuscate(code, { + ...NO_ADDITIONAL_NODES_PRESET + }).getObfuscatedCode(); + }); + + it('should correctly resolve default param to outer variable', () => { + assert.equal(evalLocal(code), 'outer'); + assert.equal(evalLocal(obfuscatedCode), evalLocal(code)); + }); + }); + }); }); diff --git a/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/destructuring-default-outer-var.js b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/destructuring-default-outer-var.js new file mode 100644 index 00000000..61480414 --- /dev/null +++ b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/destructuring-default-outer-var.js @@ -0,0 +1,8 @@ +(function() { + var x = 'outer'; + function f({ a = x } = {}) { + var x = 'inner'; + return a; + } + return f(); +})(); diff --git a/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/nested-destructuring-default-outer-var.js b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/nested-destructuring-default-outer-var.js new file mode 100644 index 00000000..bf8d3fa6 --- /dev/null +++ b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/nested-destructuring-default-outer-var.js @@ -0,0 +1,8 @@ +(function() { + var x = 'outer'; + function f({ a: { b = x } } = { a: {} }) { + var x = 'inner'; + return b; + } + return f(); +})(); diff --git a/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/simple-default-param-outer-var.js b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/simple-default-param-outer-var.js new file mode 100644 index 00000000..926fe3fe --- /dev/null +++ b/test/functional-tests/node-transformers/converting-transformers/object-pattern-properties-transformer/fixtures/simple-default-param-outer-var.js @@ -0,0 +1,8 @@ +(function() { + var x = 'outer'; + function f(a = x) { + var x = 'inner'; + return a; + } + return f(); +})(); diff --git a/test/helpers/evalLocal.ts b/test/helpers/evalLocal.ts new file mode 100644 index 00000000..237c669a --- /dev/null +++ b/test/helpers/evalLocal.ts @@ -0,0 +1,9 @@ +/** + * Evaluates code using indirect eval. + * Indirect eval runs in global scope without inheriting strict mode from the calling context. + * This is needed for testing features like Annex B function hoisting. + * + * @param {string} code + * @returns {any} + */ +export const evalLocal = (code: string): any => (0, eval)(code);