Decline proxy inlining that would duplicate an argument

src/index.ts

@@ -60,6 +60,13 @@ export function deobfuscate(source: string, parsedConfig?: Partial<Config>): str
         modifications.push(new ArrayUnpacker(ast, config.arrays.removeArrays));
     }
 
+    // replace any proxy function calls that were revealed by the array unpacking
+    if (config.proxyFunctions.replaceProxyFunctions) {
+        modifications.push(
+            new ProxyRemover(ast, config.proxyFunctions.removeProxyFunctions)
+        );
+    }
+
     // simplify any expressions that were revealed by the array unpacking
     if (config.expressions.simplifyExpressions) {
         modifications.push(new ExpressionSimplifier(ast));

src/modifications/proxies/proxyFunction.ts

@@ -7,6 +7,22 @@ import { v4 as uuid } from 'uuid';
 import Scope from '../../scope/scope';
 
 export default class ProxyFunction {
+    // A twice-used parameter of a minified MD5 helper inlines to a 9 node
+    // expression, so this budget stops that expression from duplicating again.
+    private static readonly maxDuplicableNodes = 4;
+    private static readonly sideEffectFreeTypes = new Set([
+        'BinaryExpression',
+        'ConditionalExpression',
+        'IdentifierExpression',
+        'LiteralBooleanExpression',
+        'LiteralInfinityExpression',
+        'LiteralNullExpression',
+        'LiteralNumericExpression',
+        'LiteralStringExpression',
+        'ThisExpression',
+        'UnaryExpression'
+    ]);
+
     id: string;
     node: Shift.Node;
     parentNode: Shift.Node;
@@ -65,6 +81,59 @@ export default class ProxyFunction {
         return expression;
     }
 
+    /**
+     * Returns whether a call of the proxy function can be inlined.
+     * `getReplacement` pastes an argument at every usage of its parameter, so
+     * a parameter that the body uses more than once duplicates the argument.
+     * Inlining is valid only when that argument is safe to duplicate.
+     * @param args The arguments of the proxy function call.
+     */
+    isSafeToInline(args: Shift.Expression[]): boolean {
+        const paramUsages = this.findParameterUsages(this.expression);
+
+        for (const [index, usages] of paramUsages) {
+            const arg = args[index];
+            if (usages.length > 1 && arg && !ProxyFunction.isDuplicable(arg)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    /**
+     * Returns whether an expression is safe to duplicate. It must evaluate
+     * with no side effect, so a duplicate cannot change what the program
+     * does, and it must stay under `maxDuplicableNodes`, so a duplicate
+     * cannot compound through a chain of nested calls.
+     *
+     * A call, `new`, an assignment, `++`, `--` and `delete` each have a side
+     * effect. A member expression can run a getter. A regular expression, an
+     * array, an object and a function each have their own identity, which a
+     * duplicate would split.
+     * @param node The expression node.
+     */
+    private static isDuplicable(node: Shift.Node): boolean {
+        let nodeCount = 0;
+        let duplicable = true;
+
+        traverse(node, {
+            enter(node: Shift.Node) {
+                nodeCount += 1;
+
+                if (nodeCount > ProxyFunction.maxDuplicableNodes) {
+                    duplicable = false;
+                } else if (!ProxyFunction.sideEffectFreeTypes.has(node.type)) {
+                    duplicable = false;
+                } else if (node.type == 'UnaryExpression' && node.operator == 'delete') {
+                    duplicable = false;
+                }
+            }
+        });
+
+        return duplicable;
+    }
+
     /**
      * Finds all usages of the proxy function's parameters within a given
      * expression.

src/modifications/proxies/proxyRemover.ts

@@ -22,6 +22,8 @@ export default class ProxyRemover extends Modification {
     private proxyFunctions: ProxyFunction[];
     private proxyFunctionNames: Set<string>;
     private cyclicProxyFunctionIds: Set<string>;
+    private retainedProxyFunctionIds: Set<string>;
+    private aliasDeclarations: AliasDeclaration[];
     private graph: Graph;
 
     /**
@@ -36,6 +38,8 @@ export default class ProxyRemover extends Modification {
         this.proxyFunctions = [];
         this.proxyFunctionNames = new Set<string>();
         this.cyclicProxyFunctionIds = new Set<string>();
+        this.retainedProxyFunctionIds = new Set<string>();
+        this.aliasDeclarations = [];
         this.graph = new Graph();
     }
 
@@ -47,6 +51,7 @@ export default class ProxyRemover extends Modification {
         this.findAliases();
         this.findCycles();
         this.replaceProxyFunctionUsages(this.ast, this.globalScope);
+        this.removeAliasDeclarations();
 
         if (this.shouldRemoveProxyFunctions) {
             this.removeProxyFunctions(this.globalScope);
@@ -132,7 +137,9 @@ export default class ProxyRemover extends Modification {
                         const proxyFunction = scope.get(name);
                         if (proxyFunction) {
                             scope.add(newName, proxyFunction);
-                            TraversalHelper.removeNode(parent, node);
+                            self.aliasDeclarations.push(
+                                new AliasDeclaration(parent, node, proxyFunction.id)
+                            );
                             if (!self.proxyFunctionNames.has(newName)) {
                                 self.proxyFunctionNames.add(newName);
                             }
@@ -248,13 +255,19 @@ export default class ProxyRemover extends Modification {
 
                         if (proxyFunction && !self.cyclicProxyFunctionIds.has(proxyFunction.id)) {
                             const args = (node as any).arguments;
-                            let replacement: Shift.Node = proxyFunction.getReplacement(args);
-                            replacement = self.replaceProxyFunctionUsages(replacement, scope);
 
-                            if (parent) {
-                                TraversalHelper.replaceNode(parent, node, replacement);
+                            if (!proxyFunction.isSafeToInline(args)) {
+                                // the declaration must survive, because the call stays
+                                self.retainedProxyFunctionIds.add(proxyFunction.id);
                             } else {
-                                replacedNode = replacement;
+                                let replacement: Shift.Node = proxyFunction.getReplacement(args);
+                                replacement = self.replaceProxyFunctionUsages(replacement, scope);
+
+                                if (parent) {
+                                    TraversalHelper.replaceNode(parent, node, replacement);
+                                } else {
+                                    replacedNode = replacement;
+                                }
                             }
                         }
                     }
@@ -270,13 +283,32 @@ export default class ProxyRemover extends Modification {
         return replacedNode;
     }
 
+    /**
+     * Removes the alias declarations of the proxy functions whose calls were
+     * all inlined. The alias of a retained or cyclic proxy function stays,
+     * because a call through that alias is still in the output.
+     */
+    private removeAliasDeclarations(): void {
+        for (const alias of this.aliasDeclarations) {
+            if (
+                !this.retainedProxyFunctionIds.has(alias.proxyFunctionId) &&
+                !this.cyclicProxyFunctionIds.has(alias.proxyFunctionId)
+            ) {
+                TraversalHelper.removeNode(alias.parentNode, alias.node);
+            }
+        }
+    }
+
     /**
      * Removes all proxy functions from a scope and its children.
      * @param scope The scope to remove proxy functions from.
      */
     private removeProxyFunctions(scope: Scope<ProxyFunction>): void {
         for (const [_, proxyFunction] of scope.elements) {
-            if (!this.cyclicProxyFunctionIds.has(proxyFunction.id)) {
+            if (
+                !this.cyclicProxyFunctionIds.has(proxyFunction.id) &&
+                !this.retainedProxyFunctionIds.has(proxyFunction.id)
+            ) {
                 TraversalHelper.removeNode(proxyFunction.parentNode, proxyFunction.node);
             }
         }
@@ -369,3 +401,15 @@ export default class ProxyRemover extends Modification {
         return node.type == 'CallExpression' && node.callee.type == 'IdentifierExpression';
     }
 }
+
+class AliasDeclaration {
+    parentNode: Shift.Node;
+    node: Shift.Node;
+    proxyFunctionId: string;
+
+    constructor(parentNode: Shift.Node, node: Shift.Node, proxyFunctionId: string) {
+        this.parentNode = parentNode;
+        this.node = node;
+        this.proxyFunctionId = proxyFunctionId;
+    }
+}