Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "javascript-obfuscator",
"version": "5.4.4",
"version": "5.4.5",
"description": "JavaScript obfuscator",
"keywords": [
"obfuscator",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)`
);
}
});
});
});
});
Original file line number Diff line number Diff line change
@@ -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';
})();
Loading