Fixed transformObjectKeys with mangled identifier generator causing variable shadowing when extracted object variable name matched an existing inner scope variable

CHANGELOG.md

@@ -5,6 +5,7 @@ v5.2.1
 * Fixed `transformObjectKeys` incorrectly hoisting object literal outside of loop when loop body is a single statement without braces, causing all iterations to share the same object reference. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1300
 * Fixed parsing error when `await` is used as an identifier in non-async context. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1127
 * Fixed `deadCodeInjection` causing SyntaxError when `arguments` from collected block statements was injected into class field initializers or static initialization blocks. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1166
+* Fixed `transformObjectKeys` with `mangled` identifier generator causing variable shadowing when extracted object variable name matched an existing inner scope variable. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1232
 
 v5.2.0
 ---

src/generators/identifier-names-generators/AbstractIdentifierNamesGenerator.ts

@@ -31,6 +31,11 @@ export abstract class AbstractIdentifierNamesGenerator implements IIdentifierNam
      */
     protected readonly lexicalScopesPreservedNamesMap: WeakMap<TNodeWithLexicalScope, Set<string>> = new WeakMap();
 
+    /**
+     * @type {Set<string>}
+     */
+    protected readonly allLexicalScopePreservedNames: Set<string> = new Set();
+
     /**
      * @param {IRandomGenerator} randomGenerator
      * @param {IOptions} options
@@ -72,6 +77,8 @@ export abstract class AbstractIdentifierNamesGenerator implements IIdentifierNam
         preservedNamesForLexicalScopeSet.add(name);
 
         this.lexicalScopesPreservedNamesMap.set(lexicalScopeNode, preservedNamesForLexicalScopeSet);
+
+        this.allLexicalScopePreservedNames.add(name);
     }
 
     /**
@@ -108,6 +115,23 @@ export abstract class AbstractIdentifierNamesGenerator implements IIdentifierNam
         return true;
     }
 
+    /**
+     * Checks if the name is valid and not preserved in any scope (global or lexical).
+     * This is used for global scope name generation to avoid conflicts with
+     * variables in any lexical scope that might shadow the global variable.
+     *
+     * @param {string} name
+     * @returns {boolean}
+     */
+    public isValidIdentifierNameInAllScopes(name: string): boolean {
+        if (!this.isValidIdentifierName(name)) {
+            return false;
+        }
+
+        // Check if the name is preserved in any lexical scope
+        return !this.allLexicalScopePreservedNames.has(name);
+    }
+
     /**
      * @param {string} name
      * @returns {boolean}

src/generators/identifier-names-generators/DictionaryIdentifierNamesGenerator.ts

@@ -86,7 +86,7 @@ export class DictionaryIdentifierNamesGenerator extends AbstractIdentifierNamesG
         const identifierName: string = this.generateNewDictionaryName((newIdentifierName: string) => {
             const identifierNameWithPrefix: string = `${prefix}${newIdentifierName}`;
 
-            return this.isValidIdentifierName(identifierNameWithPrefix);
+            return this.isValidIdentifierNameInAllScopes(identifierNameWithPrefix);
         });
         const identifierNameWithPrefix = `${prefix}${identifierName}`;
 

src/generators/identifier-names-generators/HexadecimalIdentifierNamesGenerator.ts

@@ -43,7 +43,7 @@ export class HexadecimalIdentifierNamesGenerator extends AbstractIdentifierNames
         const baseIdentifierName: string = hexadecimalNumber.slice(0, baseNameLength);
         const identifierName: string = `_${baseIdentifierName}`;
 
-        if (!this.isValidIdentifierName(identifierName)) {
+        if (!this.isValidIdentifierNameInAllScopes(identifierName)) {
             return this.generateNext(nameLength);
         }
 

src/generators/identifier-names-generators/MangledIdentifierNamesGenerator.ts

@@ -103,7 +103,7 @@ export class MangledIdentifierNamesGenerator extends AbstractIdentifierNamesGene
             (newIdentifierName: string) => {
                 const identifierNameWithPrefix: string = `${prefix}${newIdentifierName}`;
 
-                return this.isValidIdentifierName(identifierNameWithPrefix);
+                return this.isValidIdentifierNameInAllScopes(identifierNameWithPrefix);
             }
         );
         const identifierNameWithPrefix: string = `${prefix}${identifierName}`;

test/functional-tests/issues/fixtures/issue1232.js

@@ -0,0 +1 @@
+[].forEach(a => a === { a: 1 });

test/functional-tests/issues/issue1232.spec.ts

@@ -0,0 +1,32 @@
+import { assert } from 'chai';
+import { NO_ADDITIONAL_NODES_PRESET } from '../../../src/options/presets/NoCustomNodes';
+import { readFileAsString } from '../../helpers/readFileAsString';
+import { JavaScriptObfuscator } from '../../../src/JavaScriptObfuscatorFacade';
+
+//
+// https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1232
+//
+describe('Issue #1232', () => {
+    describe('transformObjectKeys should not cause variable shadowing with mangled identifiers', () => {
+        let obfuscatedCode: string;
+
+        before(() => {
+            const code: string = readFileAsString(__dirname + '/fixtures/issue1232.js');
+
+            obfuscatedCode = JavaScriptObfuscator.obfuscate(code, {
+                ...NO_ADDITIONAL_NODES_PRESET,
+                identifierNamesGenerator: 'mangled',
+                transformObjectKeys: true
+            }).getObfuscatedCode();
+        });
+
+        it('should not rename extracted object variable to same name as function parameter', () => {
+            const shadowingPattern = /(\w+)\s*===\s*\1[)\s;,]/;
+
+            assert.isFalse(
+                shadowingPattern.test(obfuscatedCode),
+                `Variable shadowing detected in obfuscated code: ${obfuscatedCode}`
+            );
+        });
+    });
+});