Fixed controlFlowFlattening breaking short-circuit evaluation with spread operator and conditional objects (#1373)

CHANGELOG.md

@@ -3,6 +3,7 @@ Change Log
 v5.2.0
 ---
 * Skip obfuscation of `process.env.*`
+* Fixed `controlFlowFlattening` breaking short-circuit evaluation with spread operator and conditional objects. Fixes https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1372
 
 v5.1.0
 ---

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

@@ -2,6 +2,7 @@ import { inject, injectable } from 'inversify';
 import { ServiceIdentifiers } from '../../../container/ServiceIdentifiers';
 
 import * as ESTree from 'estree';
+import * as estraverse from '@javascript-obfuscator/estraverse';
 
 import { TControlFlowCustomNodeFactory } from '../../../types/container/custom-nodes/TControlFlowCustomNodeFactory';
 import { TIdentifierNamesGeneratorFactory } from '../../../types/container/generators/TIdentifierNamesGeneratorFactory';
@@ -88,6 +89,10 @@ export class LogicalExpressionControlFlowReplacer extends ExpressionWithOperator
         leftExpression: ESTree.Expression,
         rightExpression: ESTree.Expression
     ): boolean {
+        if (this.expressionContainsProhibitedNodes(rightExpression)) {
+            return true;
+        }
+
         return [leftExpression, rightExpression].some((expressionNode: ESTree.Node | ESTree.Expression): boolean => {
             let nodeForCheck: ESTree.Node | ESTree.Expression;
 
@@ -105,4 +110,30 @@ export class LogicalExpressionControlFlowReplacer extends ExpressionWithOperator
             );
         });
     }
+
+    /**
+     * @param {Expression} expression
+     * @returns {boolean}
+     */
+    private expressionContainsProhibitedNodes(expression: ESTree.Expression): boolean {
+        let hasProhibitedNode = false;
+
+        estraverse.traverse(expression, {
+            enter: (node: ESTree.Node): estraverse.VisitorOption | void => {
+                if (NodeGuards.isMemberExpressionNode(node) && node.computed) {
+                    hasProhibitedNode = true;
+
+                    return estraverse.VisitorOption.Break;
+                }
+
+                if (NodeGuards.isCallExpressionNode(node)) {
+                    hasProhibitedNode = true;
+
+                    return estraverse.VisitorOption.Break;
+                }
+            }
+        });
+
+        return hasProhibitedNode;
+    }
 }

test/functional-tests/issues/fixtures/issue1372.js

@@ -0,0 +1,15 @@
+function test(hasColor)
+{
+  const accent = hasColor ? "primary" : undefined;
+  const theme = { palette: { primary: { main: "ok" } }};
+
+  const obj =
+  {
+    bgcolor: "test",
+    ...(accent && { color: theme.palette[accent].main })
+  }
+
+  return obj;
+}
+
+module.exports = { test };

test/functional-tests/issues/issue1372.spec.ts

@@ -0,0 +1,90 @@
+import { assert } from 'chai';
+import { NO_ADDITIONAL_NODES_PRESET } from '../../../src/options/presets/NoCustomNodes';
+import { readFileAsString } from '../../helpers/readFileAsString';
+import { JavaScriptObfuscator } from '../../../src/JavaScriptObfuscatorFacade';
+
+//
+// https://github.com/javascript-obfuscator/javascript-obfuscator/issues/1372
+//
+describe('Issue #1372', () => {
+    describe('Spread operator with conditional object should work correctly', () => {
+        const samplesCount = 50;
+
+        let obfuscatedCode: string;
+
+        it('does not break on run with controlFlowFlattening enabled', () => {
+            const code: string = readFileAsString(__dirname + '/fixtures/issue1372.js');
+
+            for (let i = 0; i < samplesCount; i++) {
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(code, {
+                    ...NO_ADDITIONAL_NODES_PRESET,
+                    controlFlowFlattening: true,
+                    controlFlowFlatteningThreshold: 1,
+                    stringArray: true,
+                    stringArrayThreshold: 1
+                }).getObfuscatedCode();
+
+                const result = eval(`
+                    ${obfuscatedCode}
+                    module.exports;
+                `);
+
+                const resultTrue = result.test(true);
+                assert.deepEqual(resultTrue, { bgcolor: 'test', color: 'ok' });
+
+                const resultFalse = result.test(false);
+                assert.deepEqual(resultFalse, { bgcolor: 'test' });
+            }
+        });
+
+        it('does not break on run with transformObjectKeys enabled', () => {
+            const code: string = readFileAsString(__dirname + '/fixtures/issue1372.js');
+
+            for (let i = 0; i < samplesCount; i++) {
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(code, {
+                    ...NO_ADDITIONAL_NODES_PRESET,
+                    transformObjectKeys: true,
+                    stringArray: true,
+                    stringArrayThreshold: 1
+                }).getObfuscatedCode();
+
+                const result = eval(`
+                    ${obfuscatedCode}
+                    module.exports;
+                `);
+
+                const resultTrue = result.test(true);
+                assert.deepEqual(resultTrue, { bgcolor: 'test', color: 'ok' });
+
+                const resultFalse = result.test(false);
+                assert.deepEqual(resultFalse, { bgcolor: 'test' });
+            }
+        });
+
+        it('does not break on run with both controlFlowFlattening and transformObjectKeys enabled', () => {
+            const code: string = readFileAsString(__dirname + '/fixtures/issue1372.js');
+
+            for (let i = 0; i < samplesCount; i++) {
+                obfuscatedCode = JavaScriptObfuscator.obfuscate(code, {
+                    ...NO_ADDITIONAL_NODES_PRESET,
+                    controlFlowFlattening: true,
+                    controlFlowFlatteningThreshold: 1,
+                    transformObjectKeys: true,
+                    stringArray: true,
+                    stringArrayThreshold: 1
+                }).getObfuscatedCode();
+
+                const result = eval(`
+                    ${obfuscatedCode}
+                    module.exports;
+                `);
+
+                const resultTrue = result.test(true);
+                assert.deepEqual(resultTrue, { bgcolor: 'test', color: 'ok' });
+
+                const resultFalse = result.test(false);
+                assert.deepEqual(resultFalse, { bgcolor: 'test' });
+            }
+        });
+    });
+});

test/functional-tests/node-transformers/preparing-transformers/obfuscating-guards/conditional-comment-obfuscating-guard/ConditionalCommentObfuscatingGuard.spec.ts

@@ -134,11 +134,11 @@ describe('ConditionalCommentObfuscatingGuard', () => {
             const ignoredFunctionExpression1RegExp: RegExp = /var bar *= *function *\(a, *b, *c\) *{/;
             const ignoredFunctionExpression2RegExp: RegExp = /var baz *= *function *\(a, *b, *c\) *{/;
 
-            const obfuscatedFunctionCallRegExp: RegExp = /_0x([a-f0-9]){5,6}\( *\);/g;
+            const obfuscatedFunctionCallRegExp: RegExp = /_0x([a-f0-9]){5,6}\( *\)[,;]/g;
             const expectedObfuscatedFunctionCallsLength: number = 3;
 
-            const ignoredFunctionCall1RegExp: RegExp = /bar\( *\);/;
-            const ignoredFunctionCall2RegExp: RegExp = /baz\( *\);/;
+            const ignoredFunctionCall1RegExp: RegExp = /bar\( *\)[,;]/;
+            const ignoredFunctionCall2RegExp: RegExp = /baz\( *\)[,;]/;
 
             let obfuscatedCode: string,
                 obfuscatedFunctionExpressionMatchesLength: number,