Fixed controlFlowFlattening intermittently dropping arguments of a spread call (e.g. foo(...args)) when it reused a control flow wrapper of a same-arity plain call (#1424)

CHANGELOG.md

@@ -1,5 +1,9 @@
 Change Log
 
+v5.4.5
+---
+* Fixed `controlFlowFlattening` intermittently dropping arguments of a spread call (e.g. `foo(...args)`) when it reused a control flow wrapper of a same-arity plain call. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1423
+
 v5.4.4
 ---
 * Optimized scope identifiers transformer performance

package.json

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

src/node-transformers/control-flow-transformers/control-flow-replacers/CallExpressionControlFlowReplacer.ts

@@ -64,14 +64,27 @@ export class CallExpressionControlFlowReplacer extends AbstractControlFlowReplac
 
         const isChainExpressionParent = NodeGuards.isChainExpressionNode(parentNode);
 
-        // Bucket reuse-eligible wrappers by both arg count AND optional-ness, so an
-        // optional `foo?.(arg)` call never reuses the wrapper of a non-optional `bar(arg)`
-        // that happens to share `arguments.length` — which would drop the `?.` short-circuit
-        // and crash on undefined callees (issue #1408).
-        const replacerId: string = `${callExpressionNode.arguments.length}-${isChainExpressionParent ? 'optional' : 'standard'}`;
+        const expressionArguments: (ESTree.Expression | ESTree.SpreadElement)[] = callExpressionNode.arguments;
+
+        // Bucket reuse-eligible wrappers by the exact argument *shape* (which positions are
+        // spread vs. plain) AND optional-ness — not just `arguments.length`.
+        //
+        // A spread call like `foo(...args)` has the same `arguments.length` as a plain
+        // `bar(arg)`, but the generated wrapper differs: `(callee, ...p1) => callee(...p1)`
+        // vs `(callee, p1) => callee(p1)`. Reusing a plain wrapper for a spread call collapses
+        // the spread to its first element and silently drops the remaining arguments (issue #1423).
+        //
+        // Encoding the per-argument shape also preserves the optional-ness bucketing that keeps
+        // an optional `foo?.(arg)` call from reusing a non-optional `bar(arg)` wrapper, which
+        // would drop the `?.` short-circuit and crash on undefined callees (issue #1408).
+        const argumentsShape: string = expressionArguments
+            .map((argument: ESTree.Expression | ESTree.SpreadElement): string =>
+                NodeGuards.isSpreadElementNode(argument) ? 's' : 'p'
+            )
+            .join('');
+        const replacerId: string = `${argumentsShape}-${isChainExpressionParent ? 'optional' : 'standard'}`;
         const callExpressionFunctionCustomNode: ICustomNode<TInitialData<CallExpressionFunctionNode>> =
             this.controlFlowCustomNodeFactory(ControlFlowCustomNode.CallExpressionFunctionNode);
-        const expressionArguments: (ESTree.Expression | ESTree.SpreadElement)[] = callExpressionNode.arguments;
 
         callExpressionFunctionCustomNode.initialize(expressionArguments, isChainExpressionParent);
 

test/functional-tests/node-transformers/control-flow-transformers/control-flow-replacers/call-expression-control-flow-replacer/CallExpressionControlFlowReplacer.spec.ts

@@ -276,5 +276,32 @@ describe('CallExpressionControlFlowReplacer', function () {
                 }
             });
         });
+
+        describe('Variant #9 - spread call must not reuse a plain wrapper of the same arity (issue #1423)', () => {
+            const samplesCount: number = 200;
+
+            it('should forward every spread-expanded argument on every obfuscation when a spread call and a plain call share the same arity', () => {
+                const code: string = readFileAsString(
+                    __dirname + '/fixtures/issue-1423-spread-and-plain-calls.js'
+                );
+
+                for (let i = 0; i < samplesCount; i++) {
+                    const obfuscatedCode: string = JavaScriptObfuscator.obfuscate(code, {
+                        ...NO_ADDITIONAL_NODES_PRESET,
+                        controlFlowFlattening: true,
+                        controlFlowFlatteningThreshold: 1,
+                        identifierNamesGenerator: 'mangled'
+                    }).getObfuscatedCode();
+
+                    const result: unknown = eval(obfuscatedCode);
+
+                    assert.strictEqual(
+                        result,
+                        'ok',
+                        `iteration ${i}: spread call reused a plain wrapper and dropped arguments (obfuscated code returned wrong value)`
+                    );
+                }
+            });
+        });
     });
 });

test/functional-tests/node-transformers/control-flow-transformers/control-flow-replacers/call-expression-control-flow-replacer/fixtures/issue-1423-spread-and-plain-calls.js

@@ -0,0 +1,26 @@
+(function () {
+    function target (a, b, c) {
+        return '' + a + b + c;
+    }
+
+    function id (x) {
+        return x;
+    }
+
+    function forward () {
+        var rest = [1, 2, 3];
+
+        // A spread-forward call `target(...rest)` has `arguments.length === 1`,
+        // the same as the plain `id(x)` calls below. It must not reuse a plain
+        // single-argument control flow wrapper, otherwise every spread-expanded
+        // argument after the first is silently dropped (issue #1423).
+        id(1);
+        id(2);
+        id(3);
+        id(4);
+
+        return target(...rest);
+    }
+
+    return forward() === '123' ? 'ok' : 'broken';
+})();