Merge pull request #562 from javascript-obfuscator/issue-550

Fixed wrong scope of preserved identifiers of shorthand object pattern property

CHANGELOG.md

@@ -1,5 +1,9 @@
 Change Log
 
+v0.25.1
+---
+* Additional fixes of https://github.com/javascript-obfuscator/javascript-obfuscator/issues/550
+
 v0.25.0
 ---
 * Improved `mangled` identifier names generator logic

index.d.ts

@@ -14,6 +14,6 @@ export interface ObfuscatedCode extends IObfuscatedCode {}
 export function obfuscate (sourceCode: string, inputOptions?: ObfuscatorOptions): ObfuscatedCode;
 
 /**
- * @type {string | undefined}
+ * @type {string}
  */
 export const version: string;

package.json

@@ -1,6 +1,6 @@
 {
   "name": "javascript-obfuscator",
-  "version": "0.25.0",
+  "version": "0.25.1",
   "description": "JavaScript obfuscator",
   "keywords": [
     "obfuscator",

src/interfaces/node/IScopeIdentifiersTraverserCallbackData.ts

@@ -4,6 +4,7 @@ import { TNodeWithLexicalScope } from '../../types/node/TNodeWithLexicalScope';
 
 export interface IScopeIdentifiersTraverserCallbackData {
     isGlobalDeclaration: boolean;
+    isBubblingDeclaration: boolean;
     rootScope: eslintScope.Scope;
     variable: eslintScope.Variable;
     variableLexicalScopeNode: TNodeWithLexicalScope;

src/node-transformers/TransformersRunner.ts

@@ -7,7 +7,6 @@ import * as ESTree from 'estree';
 
 import { TNodeTransformerFactory } from '../types/container/node-transformers/TNodeTransformerFactory';
 import { TNormalizedNodeTransformers } from '../types/node-transformers/TNormalizedNodeTransformers';
-import { TTransformersRunnerData } from '../types/node-transformers/TTransformersRunnerData';
 import { TVisitorDirection } from '../types/node-transformers/TVisitorDirection';
 import { TVisitorFunction } from '../types/node-transformers/TVisitorFunction';
 import { TVisitorResult } from '../types/node-transformers/TVisitorResult';
@@ -26,11 +25,6 @@ import { NodeMetadata } from '../node/NodeMetadata';
 
 @injectable()
 export class TransformersRunner implements ITransformersRunner {
-    /**
-     * @type {Map<NodeTransformer[], TTransformersRunnerData>}
-     */
-    private readonly cachedNodeTransformersData: Map<NodeTransformer[], TTransformersRunnerData> = new Map();
-
     /**
      * @type {TNodeTransformerFactory}
      */
@@ -70,19 +64,10 @@ export class TransformersRunner implements ITransformersRunner {
             return astTree;
         }
 
-        let normalizedNodeTransformers: TNormalizedNodeTransformers;
-        let nodeTransformerNamesGroups: NodeTransformer[][];
-
-        if (!this.cachedNodeTransformersData.has(nodeTransformerNames)) {
-            normalizedNodeTransformers = this.buildNormalizedNodeTransformers(nodeTransformerNames);
-            nodeTransformerNamesGroups = this.nodeTransformerNamesGroupsBuilder.build(normalizedNodeTransformers);
-            this.cachedNodeTransformersData.set(nodeTransformerNames, [normalizedNodeTransformers, nodeTransformerNamesGroups]);
-        } else {
-            [
-                normalizedNodeTransformers,
-                nodeTransformerNamesGroups
-            ] = <TTransformersRunnerData>this.cachedNodeTransformersData.get(nodeTransformerNames);
-        }
+        const normalizedNodeTransformers: TNormalizedNodeTransformers =
+            this.buildNormalizedNodeTransformers(nodeTransformerNames, transformationStage);
+        const nodeTransformerNamesGroups: NodeTransformer[][] =
+            this.nodeTransformerNamesGroupsBuilder.build(normalizedNodeTransformers);
 
         for (const nodeTransformerNamesGroup of nodeTransformerNamesGroups) {
             const enterVisitors: IVisitor[] = [];
@@ -120,15 +105,27 @@ export class TransformersRunner implements ITransformersRunner {
 
     /**
      * @param {NodeTransformer[]} nodeTransformerNames
+     * @param {TransformationStage} transformationStage
      * @returns {TNormalizedNodeTransformers}
      */
-    private buildNormalizedNodeTransformers (nodeTransformerNames: NodeTransformer[]): TNormalizedNodeTransformers {
+    private buildNormalizedNodeTransformers (
+        nodeTransformerNames: NodeTransformer[],
+        transformationStage: TransformationStage
+    ): TNormalizedNodeTransformers {
         return nodeTransformerNames
             .reduce<TNormalizedNodeTransformers>(
-                (acc: TNormalizedNodeTransformers, nodeTransformerName: NodeTransformer) => ({
-                    ...acc,
-                    [nodeTransformerName]: this.nodeTransformerFactory(nodeTransformerName)
-                }),
+                (acc: TNormalizedNodeTransformers, nodeTransformerName: NodeTransformer) => {
+                    const nodeTransformer: INodeTransformer = this.nodeTransformerFactory(nodeTransformerName);
+
+                    if (!nodeTransformer.getVisitor(transformationStage)) {
+                        return acc;
+                    }
+
+                    return {
+                        ...acc,
+                        [nodeTransformerName]: nodeTransformer
+                    };
+                },
                 {}
             );
     }

src/node-transformers/preparing-transformers/CommentsTransformer.ts

@@ -8,6 +8,7 @@ import { IOptions } from '../../interfaces/options/IOptions';
 import { IRandomGenerator } from '../../interfaces/utils/IRandomGenerator';
 import { IVisitor } from '../../interfaces/node-transformers/IVisitor';
 
+import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
 import { TransformationStage } from '../../enums/node-transformers/TransformationStage';
 
 import { AbstractNodeTransformer } from '../AbstractNodeTransformer';
@@ -24,6 +25,14 @@ export class CommentsTransformer extends AbstractNodeTransformer {
         '@preserve'
     ];
 
+    /**
+     * @type {NodeTransformer.ParentificationTransformer[]}
+     */
+    public readonly runAfter: NodeTransformer[] = [
+        NodeTransformer.ParentificationTransformer,
+        NodeTransformer.VariablePreserveTransformer
+    ];
+
     /**
      * @param {IRandomGenerator} randomGenerator
      * @param {IOptions} options

src/node-transformers/preparing-transformers/CustomCodeHelpersTransformer.ts

@@ -14,6 +14,7 @@ import { ICallsGraphData } from '../../interfaces/analyzers/calls-graph-analyzer
 import { IPrevailingKindOfVariablesAnalyzer } from '../../interfaces/analyzers/calls-graph-analyzer/IPrevailingKindOfVariablesAnalyzer';
 import { IVisitor } from '../../interfaces/node-transformers/IVisitor';
 
+import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
 import { ObfuscationEvent } from '../../enums/event-emitters/ObfuscationEvent';
 import { TransformationStage } from '../../enums/node-transformers/TransformationStage';
 
@@ -25,6 +26,14 @@ import { NodeGuards } from '../../node/NodeGuards';
  */
 @injectable()
 export class CustomCodeHelpersTransformer extends AbstractNodeTransformer {
+    /**
+     * @type {NodeTransformer.ParentificationTransformer[]}
+     */
+    public readonly runAfter: NodeTransformer[] = [
+        NodeTransformer.ParentificationTransformer,
+        NodeTransformer.VariablePreserveTransformer
+    ];
+
     /**
      * @type {TCustomCodeHelperGroupStorage}
      */

src/node-transformers/preparing-transformers/EvalCallExpressionTransformer.ts

@@ -8,6 +8,7 @@ import { IOptions } from '../../interfaces/options/IOptions';
 import { IRandomGenerator } from '../../interfaces/utils/IRandomGenerator';
 import { IVisitor } from '../../interfaces/node-transformers/IVisitor';
 
+import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
 import { TransformationStage } from '../../enums/node-transformers/TransformationStage';
 
 import { AbstractNodeTransformer } from '../AbstractNodeTransformer';
@@ -17,6 +18,14 @@ import { NodeUtils } from '../../node/NodeUtils';
 
 @injectable()
 export class EvalCallExpressionTransformer extends AbstractNodeTransformer {
+    /**
+     * @type {NodeTransformer.ParentificationTransformer[]}
+     */
+    public readonly runAfter: NodeTransformer[] = [
+        NodeTransformer.ParentificationTransformer,
+        NodeTransformer.VariablePreserveTransformer
+    ];
+
     /**
      * @type {Set <FunctionExpression>}
      */
@@ -147,6 +156,9 @@ export class EvalCallExpressionTransformer extends AbstractNodeTransformer {
         const evalRootAstHostNode: ESTree.FunctionExpression = NodeFactory
             .functionExpressionNode([], NodeFactory.blockStatementNode(ast));
 
+        NodeUtils.parentizeAst(evalRootAstHostNode);
+        NodeUtils.parentizeNode(evalRootAstHostNode, parentNode);
+
         /**
          * we should store that host node and then extract AST-tree on the `finalizing` stage
          */

src/node-transformers/preparing-transformers/MetadataTransformer.ts

@@ -7,6 +7,7 @@ import { IOptions } from '../../interfaces/options/IOptions';
 import { IRandomGenerator } from '../../interfaces/utils/IRandomGenerator';
 import { IVisitor } from '../../interfaces/node-transformers/IVisitor';
 
+import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
 import { TransformationStage } from '../../enums/node-transformers/TransformationStage';
 
 import { AbstractNodeTransformer } from '../AbstractNodeTransformer';
@@ -18,6 +19,14 @@ import { NodeMetadata } from '../../node/NodeMetadata';
  */
 @injectable()
 export class MetadataTransformer extends AbstractNodeTransformer {
+    /**
+     * @type {NodeTransformer.ParentificationTransformer[]}
+     */
+    public readonly runAfter: NodeTransformer[] = [
+        NodeTransformer.ParentificationTransformer,
+        NodeTransformer.VariablePreserveTransformer
+    ];
+
     /**
      * @param {IRandomGenerator} randomGenerator
      * @param {IOptions} options

src/node-transformers/preparing-transformers/ObfuscatingGuardsTransformer.ts

@@ -10,6 +10,7 @@ import { IOptions } from '../../interfaces/options/IOptions';
 import { IRandomGenerator } from '../../interfaces/utils/IRandomGenerator';
 import { IVisitor } from '../../interfaces/node-transformers/IVisitor';
 
+import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
 import { ObfuscatingGuard } from '../../enums/node-transformers/preparing-transformers/obfuscating-guards/ObfuscatingGuard';
 import { TransformationStage } from '../../enums/node-transformers/TransformationStage';
 
@@ -30,6 +31,14 @@ export class ObfuscatingGuardsTransformer extends AbstractNodeTransformer {
         ObfuscatingGuard.ReservedStringObfuscatingGuard
     ];
 
+    /**
+     * @type {NodeTransformer.ParentificationTransformer[]}
+     */
+    public readonly runAfter: NodeTransformer[] = [
+        NodeTransformer.ParentificationTransformer,
+        NodeTransformer.VariablePreserveTransformer
+    ];
+
     /**
      * @type {IObfuscatingGuard[]}
      */

src/node-transformers/preparing-transformers/VariablePreserveTransformer.ts

@@ -12,6 +12,7 @@ import { IScopeIdentifiersTraverser } from '../../interfaces/node/IScopeIdentifi
 import { IScopeIdentifiersTraverserCallbackData } from '../../interfaces/node/IScopeIdentifiersTraverserCallbackData';
 import { IVisitor } from '../../interfaces/node-transformers/IVisitor';
 
+import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
 import { ServiceIdentifiers } from '../../container/ServiceIdentifiers';
 import { TransformationStage } from '../../enums/node-transformers/TransformationStage';
 
@@ -24,6 +25,13 @@ import { NodeGuards } from '../../node/NodeGuards';
  */
 @injectable()
 export class VariablePreserveTransformer extends AbstractNodeTransformer {
+    /**
+     * @type {NodeTransformer.ParentificationTransformer[]}
+     */
+    public readonly runAfter: NodeTransformer[] = [
+        NodeTransformer.ParentificationTransformer
+    ];
+
     /**
      * @type {IIdentifierObfuscatingReplacer}
      */
@@ -53,6 +61,8 @@ export class VariablePreserveTransformer extends AbstractNodeTransformer {
             IdentifierObfuscatingReplacer.BaseIdentifierObfuscatingReplacer
         );
         this.scopeIdentifiersTraverser = scopeIdentifiersTraverser;
+
+        this.preserveScopeVariableIdentifiers = this.preserveScopeVariableIdentifiers.bind(this);
     }
 
     /**
@@ -86,36 +96,25 @@ export class VariablePreserveTransformer extends AbstractNodeTransformer {
         this.scopeIdentifiersTraverser.traverse(
             programNode,
             parentNode,
-            (data: IScopeIdentifiersTraverserCallbackData) => {
-                const {
-                    isGlobalDeclaration,
-                    variable,
-                    variableScope
-                } = data;
-
-                this.preserveScopeVariableIdentifiers(
-                    variable,
-                    variableScope,
-                    isGlobalDeclaration
-                );
-            }
+            this.preserveScopeVariableIdentifiers
         );
 
         return programNode;
     }
 
     /**
-     * @param {Variable} variable
-     * @param {Scope} variableScope
-     * @param {boolean} isGlobalDeclaration
+     * @param {IScopeIdentifiersTraverserCallbackData} data
      */
-    private preserveScopeVariableIdentifiers (
-        variable: eslintScope.Variable,
-        variableScope: eslintScope.Scope,
-        isGlobalDeclaration: boolean
-    ): void {
+    private preserveScopeVariableIdentifiers (data: IScopeIdentifiersTraverserCallbackData): void {
+        const {
+            isGlobalDeclaration,
+            isBubblingDeclaration,
+            variable,
+            variableScope
+        } = data;
+
         for (const identifier of variable.identifiers) {
-            if (isGlobalDeclaration) {
+            if (isGlobalDeclaration || isBubblingDeclaration) {
                 this.preserveIdentifierNameForRootLexicalScope(identifier);
             } else {
                 this.preserveIdentifierNameForLexicalScope(identifier, variableScope);

src/node/ScopeIdentifiersTraverser.ts

@@ -86,8 +86,17 @@ export class ScopeIdentifiersTraverser implements IScopeIdentifiersTraverser {
                 continue;
             }
 
+            const isBubblingDeclaration: boolean = variable
+                .identifiers
+                .some((identifier: ESTree.Node) =>
+                    identifier.parentNode
+                    && NodeGuards.isPropertyNode(identifier.parentNode)
+                    && identifier.parentNode.shorthand
+                );
+
             callback({
                 isGlobalDeclaration,
+                isBubblingDeclaration,
                 rootScope,
                 variable,
                 variableScope,

src/types/node-transformers/TTransformersRunnerData.ts

@@ -1,5 +0,0 @@
-import { TNormalizedNodeTransformers } from './TNormalizedNodeTransformers';
-
-import { NodeTransformer } from '../../enums/node-transformers/NodeTransformer';
-
-export type TTransformersRunnerData = [TNormalizedNodeTransformers, NodeTransformer[][]];

test/functional-tests/node-transformers/preparing-transformers/variable-preserve-transformer/VariablePreserveTransformer.spec.ts

@@ -156,4 +156,100 @@ describe('VariablePreserveTransformer', () => {
             });
         });
     });
+
+    describe('Variant #4: destructed object property identifier name conflict with identifier name', () => {
+        describe('Variant #1: `renameGlobals` option is disabled', () => {
+            const variableDeclarationIdentifierName: RegExp = /var b *= *0x1;/;
+            const destructedObjectPropertyIdentifierName: RegExp = /const { *a *} *= *{ *'a' *: *0x2 *};/;
+
+            let obfuscatedCode: string;
+
+            before(() => {
+                const code: string = readFileAsString(__dirname + '/fixtures/destructed-object-property-identifier-name-1.js');
+
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                    code,
+                    {
+                        ...NO_ADDITIONAL_NODES_PRESET,
+                        identifierNamesGenerator: 'mangled',
+                        renameGlobals: false
+                    }
+                ).getObfuscatedCode();
+            });
+
+            it('should generate non-preserved name for variable name', () => {
+                assert.match(obfuscatedCode, variableDeclarationIdentifierName);
+            });
+
+            it('should keep the original name for destructed object property identifier', () => {
+                assert.match(obfuscatedCode, destructedObjectPropertyIdentifierName);
+            });
+        });
+
+        describe('Variant #2: `renameGlobals` option is enabled', () => {
+            const variableDeclarationIdentifierName: RegExp = /var b *= *0x1;/;
+            const functionDeclarationIdentifierName: RegExp = /function c *\(\) *{/;
+            const destructedObjectPropertyIdentifierName: RegExp = /const { *a *} *= *{ *'a' *: *0x2 *};/;
+
+            let obfuscatedCode: string;
+
+            before(() => {
+                const code: string = readFileAsString(__dirname + '/fixtures/destructed-object-property-identifier-name-2.js');
+
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                    code,
+                    {
+                        ...NO_ADDITIONAL_NODES_PRESET,
+                        identifierNamesGenerator: 'mangled',
+                        renameGlobals: true
+                    }
+                ).getObfuscatedCode();
+            });
+
+            it('should generate non-preserved name for variable declaration', () => {
+                assert.match(obfuscatedCode, variableDeclarationIdentifierName);
+            });
+
+            it('should generate non-preserved name for function declaration', () => {
+                assert.match(obfuscatedCode, functionDeclarationIdentifierName);
+            });
+
+            it('should keep the original name for destructed object property identifier', () => {
+                assert.match(obfuscatedCode, destructedObjectPropertyIdentifierName);
+            });
+        });
+
+        describe('Variant #3: function destructed object property', () => {
+            const variableDeclarationIdentifierName: RegExp = /var b *= *0x1;/;
+            const destructedObjectPropertyIdentifierName: RegExp = /return *\({ *a *}\) *=> *{/;
+            const consoleLogIdentifierNames: RegExp = /console\['log']\(b, *a\);/;
+
+            let obfuscatedCode: string;
+
+            before(() => {
+                const code: string = readFileAsString(__dirname + '/fixtures/destructed-object-property-identifier-name-3.js');
+
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                    code,
+                    {
+                        ...NO_ADDITIONAL_NODES_PRESET,
+                        identifierNamesGenerator: 'mangled',
+                        renameGlobals: false
+                    }
+                ).getObfuscatedCode();
+            });
+
+            it('should generate non-preserved name for variable declaration', () => {
+                assert.match(obfuscatedCode, variableDeclarationIdentifierName);
+            });
+
+            it('should keep the original name for destructed object property identifier', () => {
+                assert.match(obfuscatedCode, destructedObjectPropertyIdentifierName);
+            });
+
+            it('should generate valid identifier names for console.log call', () => {
+                assert.match(obfuscatedCode, consoleLogIdentifierNames);
+            });
+        });
+    });
 });

test/functional-tests/node-transformers/preparing-transformers/variable-preserve-transformer/fixtures/destructed-object-property-identifier-name-1.js

@@ -0,0 +1,10 @@
+function foo() {
+    var bar = 1;
+
+    return () => {
+        const { a } = {a: 2};
+
+        console.log(bar, a);
+    };
+}
+foo();

test/functional-tests/node-transformers/preparing-transformers/variable-preserve-transformer/fixtures/destructed-object-property-identifier-name-2.js

@@ -0,0 +1,8 @@
+var foo = 1;
+
+function bar() {
+    const { a } = {a: 2};
+
+    console.log(foo, a);
+}
+bar();

test/functional-tests/node-transformers/preparing-transformers/variable-preserve-transformer/fixtures/destructed-object-property-identifier-name-3.js

@@ -0,0 +1,8 @@
+function foo() {
+    var bar = 1;
+
+    return ({a}) => {
+        console.log(bar, a);
+    }
+}
+foo();