Merge pull request #315 from javascript-obfuscator/function-obfuscation-bugs

Function obfuscation bugs

CHANGELOG.md

@@ -3,6 +3,7 @@ Change Log
 v0.18.0
 ---
 * **New option:** `reservedStrings` disables transformation of string literals, which being matched by passed RegExp patterns
+* Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/313
 * Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/309
 * Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/307
 

src/node-transformers/obfuscating-transformers/FunctionTransformer.ts

@@ -123,28 +123,27 @@ export class FunctionTransformer extends AbstractNodeTransformer {
      * @param {TNodeWithLexicalScope} lexicalScopeNode
      */
     private storeFunctionParams (functionNode: ESTree.Function, lexicalScopeNode: TNodeWithLexicalScope): void {
-        functionNode.params
-            .forEach((paramsNode: ESTree.Node) => {
-                estraverse.traverse(paramsNode, {
-                    enter: (node: ESTree.Node, parentNode: ESTree.Node | null): estraverse.VisitorOption | void => {
-                        // Should check with identifier as first argument,
-                        // because prohibited identifier can be easily ignored
-                        if (FunctionTransformer.isProhibitedIdentifierOfPropertyNode(node, parentNode)) {
-                            return;
-                        }
+        const visitor: estraverse.Visitor = {
+            enter: (node: ESTree.Node, parentNode: ESTree.Node | null): estraverse.VisitorOption | void => {
+                // Should check with identifier as first argument,
+                // because prohibited identifier can be easily ignored
+                if (FunctionTransformer.isProhibitedIdentifierOfPropertyNode(node, parentNode)) {
+                    return;
+                }
 
-                        if (NodeGuards.isAssignmentPatternNode(node) && NodeGuards.isIdentifierNode(node.left)) {
-                            this.identifierObfuscatingReplacer.storeLocalName(node.left.name, lexicalScopeNode);
+                if (NodeGuards.isAssignmentPatternNode(node) && NodeGuards.isIdentifierNode(node.left)) {
+                    this.identifierObfuscatingReplacer.storeLocalName(node.left.name, lexicalScopeNode);
 
-                            return estraverse.VisitorOption.Skip;
-                        }
+                    return estraverse.VisitorOption.Skip;
+                }
 
-                        if (NodeGuards.isIdentifierNode(node)) {
-                            this.identifierObfuscatingReplacer.storeLocalName(node.name, lexicalScopeNode);
-                        }
-                    }
-                });
-            });
+                if (NodeGuards.isIdentifierNode(node)) {
+                    this.identifierObfuscatingReplacer.storeLocalName(node.name, lexicalScopeNode);
+                }
+            }
+        };
+
+        functionNode.params.forEach((paramsNode: ESTree.Node) => estraverse.traverse(paramsNode, visitor));
     }
 
     /**
@@ -157,19 +156,12 @@ export class FunctionTransformer extends AbstractNodeTransformer {
         lexicalScopeNode: TNodeWithLexicalScope,
         ignoredIdentifierNamesSet: Set <string> = new Set()
     ): void {
-        const replaceVisitor: estraverse.Visitor = {
+        const visitor: estraverse.Visitor = {
             enter: (node: ESTree.Node, parentNode: ESTree.Node | null): void | estraverse.VisitorOption => {
-                /**
-                 * Should skip function node itself
-                 */
-                if (node === functionNode) {
-                    return;
-                }
-
                 /**
                  * Should process nested functions in different traverse loop to avoid wrong code generation
                  */
-                if (NodeGuards.isFunctionNode(node)) {
+                if (NodeGuards.isFunctionNode(node) && node !== functionNode) {
                     this.replaceFunctionParams(node, lexicalScopeNode, new Set(ignoredIdentifierNamesSet));
 
                     return estraverse.VisitorOption.Skip;
@@ -187,6 +179,7 @@ export class FunctionTransformer extends AbstractNodeTransformer {
                 if (
                     parentNode
                     && NodeGuards.isReplaceableIdentifierNode(node, parentNode)
+                    && !NodeMetadata.isRenamedIdentifier(node)
                     && !ignoredIdentifierNamesSet.has(node.name)
                 ) {
                     const newIdentifier: ESTree.Identifier = this.identifierObfuscatingReplacer
@@ -201,6 +194,6 @@ export class FunctionTransformer extends AbstractNodeTransformer {
             }
         };
 
-        estraverse.replace(functionNode, replaceVisitor)
+        estraverse.replace(functionNode, visitor)
     }
 }

test/dev/dev.ts

@@ -8,16 +8,18 @@ import { NO_ADDITIONAL_NODES_PRESET } from '../../src/options/presets/NoCustomNo
         `
         (function(foo){
             function foo () {
-            
+        
             }
-  
+        
             return new foo();
         })();
+
         `,
         {
             ...NO_ADDITIONAL_NODES_PRESET,
             compact: false,
-            transformObjectKeys: true
+            transformObjectKeys: true,
+            seed: 1
         }
     ).getObfuscatedCode();
 

test/functional-tests/node-transformers/obfuscating-transformers/function-transformer/FunctionTransformer.spec.ts

@@ -46,59 +46,107 @@ describe('FunctionTransformer', () => {
     });
 
     describe('function id name obfuscation', () => {
-        const functionExpressionParamIdentifierRegExp: RegExp = /\(function *\((_0x[a-f0-9]{4,6})\) *\{/;
-        const functionParamIdentifierRegExp: RegExp = /function *(_0x[a-f0-9]{4,6}) *\(\) *\{/;
-        const functionObjectIdentifierRegExp: RegExp = /return new (_0x[a-f0-9]{4,6}) *\(\);/;
+        describe('Variant #1', () => {
+            const functionExpressionParamIdentifierRegExp: RegExp = /\(function *\((_0x[a-f0-9]{4,6})\) *\{/;
+            const functionParamIdentifierRegExp: RegExp = /function *(_0x[a-f0-9]{4,6}) *\(\) *\{/;
+            const functionObjectIdentifierRegExp: RegExp = /return new (_0x[a-f0-9]{4,6}) *\(\);/;
 
-        let obfuscatedCode: string,
-            functionExpressionParamIdentifierName: string,
-            functionParamIdentifierName: string,
-            functionObjectIdentifierName: string;
+            let obfuscatedCode: string,
+                functionExpressionParamIdentifierName: string,
+                functionParamIdentifierName: string,
+                functionObjectIdentifierName: string;
 
-        before(() => {
-            const code: string = readFileAsString(__dirname + '/fixtures/function-id-name.js');
+            before(() => {
+                const code: string = readFileAsString(__dirname + '/fixtures/function-id-name-1.js');
 
-            obfuscatedCode = JavaScriptObfuscator.obfuscate(
-                code,
-                {
-                    ...NO_ADDITIONAL_NODES_PRESET
-                }
-            ).getObfuscatedCode();
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                    code,
+                    {
+                        ...NO_ADDITIONAL_NODES_PRESET
+                    }
+                ).getObfuscatedCode();
 
-            const functionExpressionParamIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
-                .match(functionExpressionParamIdentifierRegExp);
-            const functionParamIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
-                .match(functionParamIdentifierRegExp);
-            const functionObjectIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
-                .match(functionObjectIdentifierRegExp);
+                const functionExpressionParamIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
+                    .match(functionExpressionParamIdentifierRegExp);
+                const functionParamIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
+                    .match(functionParamIdentifierRegExp);
+                const functionObjectIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
+                    .match(functionObjectIdentifierRegExp);
 
-            functionParamIdentifierName = (<RegExpMatchArray>functionParamIdentifierMatch)[1];
-            functionExpressionParamIdentifierName = (<RegExpMatchArray>functionExpressionParamIdentifierMatch)[1];
-            functionObjectIdentifierName = (<RegExpMatchArray>functionObjectIdentifierMatch)[1];
-        });
+                functionParamIdentifierName = (<RegExpMatchArray>functionParamIdentifierMatch)[1];
+                functionExpressionParamIdentifierName = (<RegExpMatchArray>functionExpressionParamIdentifierMatch)[1];
+                functionObjectIdentifierName = (<RegExpMatchArray>functionObjectIdentifierMatch)[1];
+            });
 
-        it('should correctly transform function expression parameter identifier', () => {
-            assert.match(obfuscatedCode, functionExpressionParamIdentifierRegExp);
-        });
+            it('should correctly transform function expression parameter identifier', () => {
+                assert.match(obfuscatedCode, functionExpressionParamIdentifierRegExp);
+            });
 
-        it('should correctly transform function parameter identifier', () => {
-            assert.match(obfuscatedCode, functionParamIdentifierRegExp);
-        });
+            it('should correctly transform function parameter identifier', () => {
+                assert.match(obfuscatedCode, functionParamIdentifierRegExp);
+            });
 
-        it('should correctly transform function object parameter identifier', () => {
-            assert.match(obfuscatedCode, functionObjectIdentifierRegExp);
-        });
+            it('should correctly transform function object parameter identifier', () => {
+                assert.match(obfuscatedCode, functionObjectIdentifierRegExp);
+            });
 
-        it('should generate same names for function parameter and function object identifiers', () => {
-            assert.equal(functionParamIdentifierName, functionObjectIdentifierName);
-        });
+            it('should generate same names for function parameter and function object identifiers', () => {
+                assert.equal(functionParamIdentifierName, functionObjectIdentifierName);
+            });
+
+            it('should generate same names for function parameter identifiers', () => {
+                assert.equal(functionExpressionParamIdentifierName, functionParamIdentifierName);
+            });
 
-        it('should generate same names for function parameter identifiers', () => {
-            assert.equal(functionExpressionParamIdentifierName, functionParamIdentifierName);
+            it('should generate same names for function expression parameter and function object identifiers', () => {
+                assert.equal(functionExpressionParamIdentifierName, functionObjectIdentifierName);
+            });
         });
 
-        it('should generate same names for function expression parameter and function object identifiers', () => {
-            assert.equal(functionExpressionParamIdentifierName, functionObjectIdentifierName);
+        describe('Variant #2', () => {
+            const functionIdentifiersRegExp: RegExp = /function *(_0x[a-f0-9]{4,6}) *\((_0x[a-f0-9]{4,6})\) *\{/;
+            const functionObjectIdentifierRegExp: RegExp = /return new (_0x[a-f0-9]{4,6}) *\(\);/;
+
+            let obfuscatedCode: string,
+                functionIdentifierName: string,
+                functionParamIdentifierName: string,
+                functionObjectIdentifierName: string;
+
+            before(() => {
+                const code: string = readFileAsString(__dirname + '/fixtures/function-id-name-2.js');
+
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(
+                    code,
+                    {
+                        ...NO_ADDITIONAL_NODES_PRESET
+                    }
+                ).getObfuscatedCode();
+
+                const functionIdentifiersMatch: RegExpMatchArray|null = obfuscatedCode
+                    .match(functionIdentifiersRegExp);
+                const functionObjectIdentifierMatch: RegExpMatchArray|null = obfuscatedCode
+                    .match(functionObjectIdentifierRegExp);
+
+                functionIdentifierName = (<RegExpMatchArray>functionIdentifiersMatch)[1];
+                functionParamIdentifierName = (<RegExpMatchArray>functionIdentifiersMatch)[2];
+                functionObjectIdentifierName = (<RegExpMatchArray>functionObjectIdentifierMatch)[1];
+            });
+
+            it('should correctly transform function identifiers', () => {
+                assert.match(obfuscatedCode, functionIdentifiersRegExp);
+            });
+
+            it('should correctly transform function object parameter identifier', () => {
+                assert.match(obfuscatedCode, functionObjectIdentifierRegExp);
+            });
+
+            it('should generate same names for function parameter and function object identifiers', () => {
+                assert.equal(functionIdentifierName, functionObjectIdentifierName);
+            });
+
+            it('should generate same names for function id and parameter identifiers', () => {
+                assert.equal(functionIdentifierName, functionParamIdentifierName);
+            });
         });
     });
 
@@ -356,27 +404,27 @@ describe('FunctionTransformer', () => {
                 assert.match(obfuscatedCode, functionBodyRegExp);
             });
 
-            it('equal #1: shouldn\'t keep same names for variable declaration identifier and function parameters identifiers', () => {
-                assert.notEqual(variableDeclarationIdentifierName, functionParameterIdentifierName);
+            it('equal #1: should keep same names for variable declaration identifier and function parameters identifiers', () => {
+                assert.equal(variableDeclarationIdentifierName, functionParameterIdentifierName);
             });
 
             it('equal #2: shouldn\'t keep same names for variable declaration identifier and function parameters identifiers', () => {
                 assert.notEqual(variableDeclarationIdentifierName, functionDefaultParameterIdentifierName1);
             });
 
-            it('equal #3: shouldn\'t keep same names for variable declaration identifier and function parameters identifiers', () => {
-                assert.notEqual(variableDeclarationIdentifierName, functionDefaultParameterIdentifierName2);
+            it('equal #3: should keep same names for variable declaration identifier and function parameters identifiers', () => {
+                assert.equal(variableDeclarationIdentifierName, functionDefaultParameterIdentifierName2);
             });
 
-            it('should keep same names for identifier in first function parameter and default value identifier of second function parameter', () => {
+            it('equal #4: should keep same names for identifier in first function parameter and default value identifier of second function parameter', () => {
                 assert.equal(functionParameterIdentifierName, functionDefaultParameterIdentifierName2);
             });
 
-            it('equal #1: should keep same names for identifiers in function params and function body', () => {
+            it('equal #5: should keep same names for identifiers in function params and function body', () => {
                 assert.equal(functionParameterIdentifierName, functionBodyIdentifierName1);
             });
 
-            it('equal #2: should keep same names for identifiers in function params and function body', () => {
+            it('equal #6: should keep same names for identifiers in function params and function body', () => {
                 assert.equal(functionDefaultParameterIdentifierName1, functionBodyIdentifierName2);
             });
         });

test/functional-tests/node-transformers/obfuscating-transformers/function-transformer/fixtures/function-id-name-2.js

@@ -0,0 +1,5 @@
+(function () {
+    function foo (foo) {}
+
+    return new foo();
+})();

test/functional-tests/node-transformers/obfuscating-transformers/variable-declaration-transformer/VariableDeclarationTransformer.spec.ts

@@ -162,13 +162,13 @@ describe('VariableDeclarationTransformer', () => {
     describe(`Variant #6: variable calls before variable declaration when function param has the same name as variables name`, () => {
         const functionParamIdentifierRegExp: RegExp = /function *_0x[a-f0-9]{4,6} *\((_0x[a-f0-9]{4,6})\,(_0x[a-f0-9]{4,6})\) *\{/;
         const innerFunctionParamIdentifierRegExp: RegExp = /function _0x[a-f0-9]{4,6} *\((_0x[a-f0-9]{4,6})\) *\{/;
-        const constructorIdentifierRegExp: RegExp = /console\['log'\]\((_0x[a-f0-9]{4,6})\)/;
+        const consoleLogIdentifierRegExp: RegExp = /console\['log'\]\((_0x[a-f0-9]{4,6})\)/;
         const objectIdentifierRegExp: RegExp = /return\{'t':(_0x[a-f0-9]{4,6})\}/;
         const variableDeclarationIdentifierRegExp: RegExp = /var *(_0x[a-f0-9]{4,6});/;
 
         let outerFunctionParamIdentifierName: string|null,
             innerFunctionParamIdentifierName: string|null,
-            constructorIdentifierName: string|null,
+            consoleLogIdentifierName: string|null,
             objectIdentifierName: string|null,
             variableDeclarationIdentifierName: string|null;
 
@@ -184,29 +184,29 @@ describe('VariableDeclarationTransformer', () => {
 
             outerFunctionParamIdentifierName = getRegExpMatch(obfuscatedCode, functionParamIdentifierRegExp);
             innerFunctionParamIdentifierName = getRegExpMatch(obfuscatedCode, innerFunctionParamIdentifierRegExp);
-            constructorIdentifierName = getRegExpMatch(obfuscatedCode, constructorIdentifierRegExp);
+            consoleLogIdentifierName = getRegExpMatch(obfuscatedCode, consoleLogIdentifierRegExp);
             objectIdentifierName = getRegExpMatch(obfuscatedCode, objectIdentifierRegExp);
             variableDeclarationIdentifierName = getRegExpMatch(obfuscatedCode, variableDeclarationIdentifierRegExp);
         });
 
-        it('match #1: should\'t name variables inside inner function with names from outer function params', () => {
-            assert.notEqual(outerFunctionParamIdentifierName, constructorIdentifierName);
+        it('equal #1: should generate same names for identifiers inside inner function and outer function params', () => {
+            assert.equal(outerFunctionParamIdentifierName, consoleLogIdentifierName);
         });
 
-        it('match #2: should\'t name variables inside inner function with names from outer function params', () => {
-            assert.notEqual(outerFunctionParamIdentifierName, innerFunctionParamIdentifierName);
+        it('equal #2: should generate same names for identifiers inside inner function and inner function params', () => {
+            assert.equal(outerFunctionParamIdentifierName, innerFunctionParamIdentifierName);
         });
 
-        it('match #1: should correct transform variables inside outer function body', () => {
+        it('equal #1: should correct transform variables inside outer function body', () => {
             assert.equal(outerFunctionParamIdentifierName, objectIdentifierName);
         });
 
-        it('match #2: should correct transform variables inside outer function body', () => {
+        it('equal #2: should correct transform variables inside outer function body', () => {
             assert.equal(outerFunctionParamIdentifierName, variableDeclarationIdentifierName);
         });
 
         it('should correct transform variables inside inner function body', () => {
-            assert.equal(innerFunctionParamIdentifierName, constructorIdentifierName);
+            assert.equal(innerFunctionParamIdentifierName, consoleLogIdentifierName);
         });
 
         it('should keep equal names after transformation for variables with same names', () => {
@@ -217,13 +217,13 @@ describe('VariableDeclarationTransformer', () => {
     describe(`Variant #7: variable calls before variable declaration when catch clause param has the same name as variables name`, () => {
         const catchClauseParamIdentifierRegExp: RegExp = /catch *\((_0x[a-f0-9]{4,6})\) *\{/;
         const innerFunctionParamIdentifierRegExp: RegExp = /function _0x[a-f0-9]{4,6} *\((_0x[a-f0-9]{4,6})\) *\{/;
-        const constructorIdentifierRegExp: RegExp = /console\['log'\]\((_0x[a-f0-9]{4,6})\)/;
+        const consoleLogIdentifierRegExp: RegExp = /console\['log'\]\((_0x[a-f0-9]{4,6})\)/;
         const objectIdentifierRegExp: RegExp = /return\{'t':(_0x[a-f0-9]{4,6})\}/;
         const variableDeclarationIdentifierRegExp: RegExp = /var *(_0x[a-f0-9]{4,6});/;
 
         let catchClauseParamIdentifierName: string|null,
             innerFunctionParamIdentifierName: string|null,
-            constructorIdentifierName: string|null,
+            consoleLogIdentifierName: string|null,
             objectIdentifierName: string|null,
             variableDeclarationIdentifierName: string|null;
 
@@ -239,17 +239,17 @@ describe('VariableDeclarationTransformer', () => {
 
             catchClauseParamIdentifierName = getRegExpMatch(obfuscatedCode, catchClauseParamIdentifierRegExp);
             innerFunctionParamIdentifierName = getRegExpMatch(obfuscatedCode, innerFunctionParamIdentifierRegExp);
-            constructorIdentifierName = getRegExpMatch(obfuscatedCode, constructorIdentifierRegExp);
+            consoleLogIdentifierName = getRegExpMatch(obfuscatedCode, consoleLogIdentifierRegExp);
             objectIdentifierName = getRegExpMatch(obfuscatedCode, objectIdentifierRegExp);
             variableDeclarationIdentifierName = getRegExpMatch(obfuscatedCode, variableDeclarationIdentifierRegExp);
         });
 
-        it('match #1: should\'t name variables inside inner function with names from catch clause param', () => {
-            assert.notEqual(catchClauseParamIdentifierName, constructorIdentifierName);
+        it('match #1: should generate same names for identifiers inside inner function and catch clause param', () => {
+            assert.equal(catchClauseParamIdentifierName, consoleLogIdentifierName);
         });
 
-        it('match #2: should\'t name variables inside inner function with names from catch clause param', () => {
-            assert.notEqual(catchClauseParamIdentifierName, innerFunctionParamIdentifierName);
+        it('match #2: should generate same names for identifiers inside inner function and catch clause param', () => {
+            assert.equal(catchClauseParamIdentifierName, innerFunctionParamIdentifierName);
         });
 
         it('equal #1: should correct transform variables inside catch clause body', () => {
@@ -261,7 +261,7 @@ describe('VariableDeclarationTransformer', () => {
         });
 
         it('should correct transform variables inside inner function body', () => {
-            assert.equal(innerFunctionParamIdentifierName, constructorIdentifierName);
+            assert.equal(innerFunctionParamIdentifierName, consoleLogIdentifierName);
         });
 
         it('should keep equal names after transformation for variables with same names', () => {