Merge pull request #524 from javascript-obfuscator/transform-object-keys-prohibited-host-fix

`transformObjectKeys` refactoring and bug fixes

CHANGELOG.md

@@ -6,6 +6,7 @@ v0.24.0
 * Dynamic import and `import.meta` support. Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/505
 * Now usage of some browser-related options with `target: 'node'` will cause a validation error
 * **CLI:** a file path will be displayed on obfuscation error. Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/513
+* Fixed many `transformObjectKeys` runtime errors
 * Fixed `Maximum call stack size exceeded` error on large strings when `splitString` option is enabled
 * Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/516
 * Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/512

package.json

@@ -1,6 +1,6 @@
 {
   "name": "javascript-obfuscator",
-  "version": "0.24.0-rc.3",
+  "version": "0.24.0-rc.4",
   "description": "JavaScript obfuscator",
   "keywords": [
     "obfuscator",

src/enums/node/NodeType.ts

@@ -11,6 +11,7 @@ export enum NodeType {
     CallExpression = 'CallExpression',
     CatchClause = 'CatchClause',
     ClassDeclaration = 'ClassDeclaration',
+    ConditionalExpression = 'ConditionalExpression',
     ContinueStatement = 'ContinueStatement',
     ExportNamedDeclaration = 'ExportNamedDeclaration',
     ExpressionStatement = 'ExpressionStatement',

src/node-transformers/converting-transformers/ObjectExpressionKeysTransformer.ts

@@ -57,161 +57,55 @@ export class ObjectExpressionKeysTransformer extends AbstractNodeTransformer {
         objectExpressionNode: ESTree.ObjectExpression,
         hostStatement: ESTree.Statement
     ): boolean {
-        return ObjectExpressionKeysTransformer.isProhibitedVariableDeclarationHostStatement(objectExpressionNode, hostStatement)
-            || ObjectExpressionKeysTransformer.isProhibitedFunctionHostStatement(objectExpressionNode, hostStatement)
-            || ObjectExpressionKeysTransformer.isProhibitedExpressionHostStatement(objectExpressionNode, hostStatement)
-            || ObjectExpressionKeysTransformer.isProhibitedReturnHostStatement(objectExpressionNode, hostStatement);
-    }
-
-    /**
-     * Fix of https://github.com/javascript-obfuscator/javascript-obfuscator/issues/516
-     * If object expression is placed inside any expression inside variable declaration with 2+ declarators
-     * - should mark host node as prohibited
-     *
-     * @param {ObjectExpression} objectExpressionNode
-     * @param {Statement} hostStatement
-     * @returns {boolean}
-     */
-    private static isProhibitedVariableDeclarationHostStatement (
-        objectExpressionNode: ESTree.ObjectExpression,
-        hostStatement: ESTree.Statement
-    ): boolean {
-        if (!NodeGuards.isVariableDeclarationNode(hostStatement) || !hostStatement.declarations.length) {
-            return false;
-        }
-
-        return ObjectExpressionKeysTransformer.isReferencedIdentifierName(
-            objectExpressionNode,
-            hostStatement.declarations
-        );
-    }
-
-    /**
-     * @param {ObjectExpression} objectExpressionNode
-     * @param {Statement} hostStatement
-     * @returns {boolean}
-     */
-    private static isProhibitedFunctionHostStatement (
-        objectExpressionNode: ESTree.ObjectExpression,
-        hostStatement: ESTree.Statement
-    ): boolean {
-        if (!NodeGuards.isFunctionNode(hostStatement) || !hostStatement.params.length) {
-            return false;
-        }
-
-        const hostNode: ESTree.Node | undefined = objectExpressionNode.parentNode;
-
-        if (!hostNode || !NodeGuards.isAssignmentPatternNode(hostNode)) {
-            return false;
-        }
-
-        return ObjectExpressionKeysTransformer.isReferencedIdentifierName(
-            objectExpressionNode,
-            hostStatement.params
-        );
-    }
-
-    /**
-     * @param {ObjectExpression} objectExpressionNode
-     * @param {Statement} hostStatement
-     * @returns {boolean}
-     */
-    private static isProhibitedReturnHostStatement (
-        objectExpressionNode: ESTree.ObjectExpression,
-        hostStatement: ESTree.Statement
-    ): boolean {
-        if (!NodeGuards.isReturnStatementNode(hostStatement) || !hostStatement.argument) {
-            return false;
-        }
-
-        return ObjectExpressionKeysTransformer.isProhibitedSequenceExpressionNode(
-            objectExpressionNode,
-            hostStatement.argument
-        );
-    }
-
-    /**
-     * @param {ObjectExpression} objectExpressionNode
-     * @param {Statement} hostStatement
-     * @returns {boolean}
-     */
-    private static isProhibitedExpressionHostStatement (
-        objectExpressionNode: ESTree.ObjectExpression,
-        hostStatement: ESTree.Statement
-    ): boolean {
-        if (!NodeGuards.isExpressionStatementNode(hostStatement)) {
-            return false;
-        }
-
-        return ObjectExpressionKeysTransformer.isProhibitedSequenceExpressionNode(
-            objectExpressionNode,
-            hostStatement.expression
-        );
-    }
-
-    /**
-     * @param {ObjectExpression} objectExpressionNode
-     * @param {Statement} node
-     * @returns {boolean}
-     */
-    private static isProhibitedSequenceExpressionNode (
-        objectExpressionNode: ESTree.ObjectExpression,
-        node: ESTree.Node
-    ): boolean {
-        if (!NodeGuards.isSequenceExpressionNode(node) || !node.expressions.length) {
-            return false;
-        }
-
         return ObjectExpressionKeysTransformer.isReferencedIdentifierName(
             objectExpressionNode,
-            node.expressions
+            hostStatement
         );
     }
 
     /**
      * @param {ObjectExpression} objectExpressionNode
-     * @param {Node[]} nodesToSearch
+     * @param {Node} hostNode
      * @returns {boolean}
      */
     private static isReferencedIdentifierName (
         objectExpressionNode: ESTree.ObjectExpression,
-        nodesToSearch: ESTree.Node[],
+        hostNode: ESTree.Node,
     ): boolean {
-        if (nodesToSearch.length === 1) {
-            return false;
-        }
-
         const identifierNamesSet: string[] = [];
 
         let isReferencedIdentifierName: boolean = false;
         let isCurrentNode: boolean = false;
 
         // should mark node as prohibited if identifier of node is referenced somewhere inside other nodes
-        for (const nodeToSearch of nodesToSearch) {
-            estraverse.traverse(nodeToSearch, {
-                enter: (node: ESTree.Node): void | estraverse.VisitorOption => {
-                    if (node === objectExpressionNode) {
-                        isCurrentNode = true;
-                    }
+        estraverse.traverse(hostNode, {
+            enter: (node: ESTree.Node): void | estraverse.VisitorOption => {
+                if (node === objectExpressionNode) {
+                    isCurrentNode = true;
+                }
 
-                    if (!NodeGuards.isIdentifierNode(node)) {
-                        return;
-                    }
+                if (!NodeGuards.isIdentifierNode(node)) {
+                    return;
+                }
 
-                    if (!isCurrentNode) {
-                        identifierNamesSet.push(node.name);
-                    } else if (identifierNamesSet.includes(node.name)) {
-                        isReferencedIdentifierName = true;
+                if (!isCurrentNode) {
+                    identifierNamesSet.push(node.name);
 
-                        return estraverse.VisitorOption.Break;
-                    }
+                    return;
+                }
+
+                if (identifierNamesSet.includes(node.name)) {
+                    isReferencedIdentifierName = true;
                 }
-            });
 
-            if (isCurrentNode || isReferencedIdentifierName) {
-                break;
+                return estraverse.VisitorOption.Break;
+            },
+            leave: (node: ESTree.Node): void | estraverse.VisitorOption => {
+                if (node === objectExpressionNode) {
+                    isCurrentNode = false;
+                }
             }
-        }
+        });
 
         return isReferencedIdentifierName;
     }

src/node/NodeGuards.ts

@@ -358,14 +358,6 @@ export class NodeGuards {
         return node.type === NodeType.ReturnStatement;
     }
 
-    /**
-     * @param {Node} node
-     * @returns {boolean}
-     */
-    public static isSequenceExpressionNode (node: ESTree.Node): node is ESTree.SequenceExpression {
-        return node.type === NodeType.SequenceExpression;
-    }
-
     /**
      * @param {Node} node
      * @returns {boolean}

test/dev/dev.ts

@@ -9,10 +9,12 @@ import { NO_ADDITIONAL_NODES_PRESET } from '../../src/options/presets/NoCustomNo
         `
             function test() {
                 var foo;
-                return (foo = {props: 1})['state'] = {
-                    expanded: foo.props
-                },
-                foo.state.expanded;
+                
+                return foo
+                    ? {
+                        bar: foo.foo
+                    }
+                    : 1;
             }
             
             console.log(test());

test/functional-tests/node-transformers/converting-transformers/object-expression-keys-transformer/ObjectExpressionKeysTransformer.spec.ts

@@ -1549,7 +1549,7 @@ describe('ObjectExpressionKeysTransformer', () => {
                     ).getObfuscatedCode();
                 });
 
-                it('shouldn ignore sequence expression object expression if it references same sequence expression identifier', () => {
+                it('shouldn ignore sequence expression object expression if it references other sequence expression identifier', () => {
                     assert.match(obfuscatedCode,  regExp);
                 });
             });
@@ -1557,7 +1557,9 @@ describe('ObjectExpressionKeysTransformer', () => {
             describe('Variant #2: reference on same sequence expression identifier', () => {
                 const match: string = `` +
                     `var ${variableMatch};` +
-                    `return *\\(${variableMatch} *= *{'props' *: *0x1}\\)\\['state'] *= *{'expanded' *: *${variableMatch}\\['props']}, *` +
+                    `var ${variableMatch} *= *{};` +
+                    `${variableMatch}\\['props'] *= *0x1;` +
+                    `return *\\(${variableMatch} *= *${variableMatch}\\)\\['state'] *= *{'expanded' *: *${variableMatch}\\['props']}, *` +
                     `${variableMatch}\\['state']\\['expanded'];` +
                 ``;
                 const regExp: RegExp = new RegExp(match);
@@ -1576,7 +1578,122 @@ describe('ObjectExpressionKeysTransformer', () => {
                     ).getObfuscatedCode();
                 });
 
-                it('shouldn ignore sequence expression object expression if it references same sequence expression identifier', () => {
+                it('shouldn ignore sequence expression object expression if it references other sequence expression identifier', () => {
+                    assert.match(obfuscatedCode,  regExp);
+                });
+            });
+        });
+
+        describe('Variant #7: conditional expression identifier reference', () => {
+            describe('Variant #1: conditional expression identifier reference', () => {
+                const match: string = `` +
+                    `var ${variableMatch};` +
+                    `var ${variableMatch} *= *{};` +
+                    `${variableMatch}\\['bar'] *= *0x1;` +
+                    `${variableMatch} *\\? *{'bar' *: *${variableMatch}\\['foo']} *: *${variableMatch};` +
+                ``;
+                const regExp: RegExp = new RegExp(match);
+
+                let obfuscatedCode: string;
+
+                before(() => {
+                    const code: string = readFileAsString(__dirname + '/fixtures/conditional-expression-identifier-reference.js');
+
+                    obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                        code,
+                        {
+                            ...NO_ADDITIONAL_NODES_PRESET,
+                            transformObjectKeys: true
+                        }
+                    ).getObfuscatedCode();
+                });
+
+                it('shouldn ignore conditional expression object expression if it references other conditional expression identifier', () => {
+                    assert.match(obfuscatedCode,  regExp);
+                });
+            });
+
+            describe('Variant #2: return statement conditional expression identifier reference', () => {
+                const match: string = `` +
+                    `var ${variableMatch};` +
+                    `var ${variableMatch} *= *{};` +
+                    `${variableMatch}\\['bar'] *= *0x1;` +
+                    `return ${variableMatch} *\\? *{'bar' *: *${variableMatch}\\['foo']} *: *${variableMatch};` +
+                ``;
+                const regExp: RegExp = new RegExp(match);
+
+                let obfuscatedCode: string;
+
+                before(() => {
+                    const code: string = readFileAsString(__dirname + '/fixtures/return-statement-conditional-expression-identifier-reference.js');
+
+                    obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                        code,
+                        {
+                            ...NO_ADDITIONAL_NODES_PRESET,
+                            transformObjectKeys: true
+                        }
+                    ).getObfuscatedCode();
+                });
+
+                it('shouldn ignore conditional expression object expression if it references other conditional expression identifier', () => {
+                    assert.match(obfuscatedCode,  regExp);
+                });
+            });
+
+            describe('Variant #3: assignment expression conditional expression identifier reference', () => {
+                const match: string = `` +
+                    `var ${variableMatch};` +
+                    `var ${variableMatch};` +
+                    `var ${variableMatch} *= *{};` +
+                    `${variableMatch}\\['bar'] *= *0x1;` +
+                    `${variableMatch} *= *${variableMatch} *\\? *{'bar' *: *${variableMatch}\\['foo']} *: *${variableMatch};` +
+                ``;
+                const regExp: RegExp = new RegExp(match);
+
+                let obfuscatedCode: string;
+
+                before(() => {
+                    const code: string = readFileAsString(__dirname + '/fixtures/assignment-expression-conditional-expression-identifier-reference.js');
+
+                    obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                        code,
+                        {
+                            ...NO_ADDITIONAL_NODES_PRESET,
+                            transformObjectKeys: true
+                        }
+                    ).getObfuscatedCode();
+                });
+
+                it('shouldn ignore conditional expression object expression if it references other conditional expression identifier', () => {
+                    assert.match(obfuscatedCode,  regExp);
+                });
+            });
+
+            describe('Variant #4: variable declarator conditional expression identifier reference', () => {
+                const match: string = `` +
+                    `var ${variableMatch};` +
+                    `var ${variableMatch} *= *{};` +
+                    `${variableMatch}\\['bar'] *= *0x1;` +
+                    `var ${variableMatch} *= *${variableMatch} *\\? *{'bar' *: *${variableMatch}\\['foo']} *: *${variableMatch};` +
+                ``;
+                const regExp: RegExp = new RegExp(match);
+
+                let obfuscatedCode: string;
+
+                before(() => {
+                    const code: string = readFileAsString(__dirname + '/fixtures/variable-declarator-conditional-expression-identifier-reference.js');
+
+                    obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                        code,
+                        {
+                            ...NO_ADDITIONAL_NODES_PRESET,
+                            transformObjectKeys: true
+                        }
+                    ).getObfuscatedCode();
+                });
+
+                it('shouldn ignore conditional expression object expression if it references other conditional expression identifier', () => {
                     assert.match(obfuscatedCode,  regExp);
                 });
             });

test/functional-tests/node-transformers/converting-transformers/object-expression-keys-transformer/fixtures/assignment-expression-conditional-expression-identifier-reference.js

@@ -0,0 +1,12 @@
+function test() {
+    var foo;
+    var bar;
+
+    bar = foo
+        ? {
+            bar: foo.foo
+        }
+        : {
+            bar: 1
+        };
+}

test/functional-tests/node-transformers/converting-transformers/object-expression-keys-transformer/fixtures/conditional-expression-identifier-reference.js

@@ -0,0 +1,11 @@
+function test() {
+    var foo;
+
+    foo
+        ? {
+            bar: foo.foo
+        }
+        : {
+            bar: 1
+        };
+}

test/functional-tests/node-transformers/converting-transformers/object-expression-keys-transformer/fixtures/return-statement-conditional-expression-identifier-reference.js

@@ -0,0 +1,11 @@
+function test() {
+    var foo;
+
+    return foo
+        ? {
+            bar: foo.foo
+        }
+        : {
+            bar: 1
+        };
+}

test/functional-tests/node-transformers/converting-transformers/object-expression-keys-transformer/fixtures/variable-declarator-conditional-expression-identifier-reference.js

@@ -0,0 +1,11 @@
+function test() {
+    var foo;
+
+    var bar = foo
+        ? {
+            bar: foo.foo
+        }
+        : {
+            bar: 1
+        };
+}