Merge pull request #541 from javascript-obfuscator/rc4-encoded-value-collision-fix

Fix of rc4 encoded values collision

CHANGELOG.md

@@ -1,5 +1,9 @@
 Change Log
 
+v0.24.4
+---
+* Fixed rc4 encoded value collision: https://github.com/javascript-obfuscator/javascript-obfuscator/issues/538
+
 v0.24.3
 ---
 * Fixed https://github.com/javascript-obfuscator/javascript-obfuscator/issues/535

package.json

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

src/analyzers/string-array-storage-analyzer/StringArrayStorageAnalyzer.ts

@@ -50,7 +50,7 @@ export class StringArrayStorageAnalyzer implements IStringArrayStorageAnalyzer {
      * @param {IOptions} options
      */
     public constructor (
-        @inject(ServiceIdentifiers.TStringArrayStorage) stringArrayStorage: IStringArrayStorage,
+        @inject(ServiceIdentifiers.IStringArrayStorage) stringArrayStorage: IStringArrayStorage,
         @inject(ServiceIdentifiers.IRandomGenerator) randomGenerator: IRandomGenerator,
         @inject(ServiceIdentifiers.IOptions) options: IOptions,
     ) {

src/container/ServiceIdentifiers.ts

@@ -42,11 +42,11 @@ export enum ServiceIdentifiers {
     ISourceCode = 'ISourceCode',
     ISourceMapCorrector = 'ISourceMapCorrector',
     IScopeAnalyzer = 'IScopeAnalyzer',
+    IStringArrayStorage = 'IStringArrayStorage',
     IStringArrayStorageAnalyzer = 'IStringArrayStorageAnalyzer',
     ITransformersRunner = 'ITransformersRunner',
     Newable__ICustomNode = 'Newable<ICustomNode>',
     Newable__TControlFlowStorage = 'Newable<TControlFlowStorage>',
     TCustomNodeGroupStorage = 'TCustomNodeGroupStorage',
-    TInputOptions = 'TInputOptions',
-    TStringArrayStorage = 'TStringArrayStorage'
+    TInputOptions = 'TInputOptions'
 }

src/container/modules/storages/StoragesModule.ts

@@ -18,7 +18,7 @@ export const storagesModule: interfaces.ContainerModule = new ContainerModule((b
         .to(CustomNodeGroupStorage)
         .inSingletonScope();
 
-    bind<IStringArrayStorage>(ServiceIdentifiers.TStringArrayStorage)
+    bind<IStringArrayStorage>(ServiceIdentifiers.IStringArrayStorage)
         .to(StringArrayStorage)
         .inSingletonScope();
 

src/custom-nodes/string-array-nodes/group/StringArrayCustomNodeGroup.ts

@@ -55,7 +55,7 @@ export class StringArrayCustomNodeGroup extends AbstractCustomNodeGroup {
      */
     public constructor (
         @inject(ServiceIdentifiers.Factory__ICustomNode) customNodeFactory: TCustomNodeFactory,
-        @inject(ServiceIdentifiers.TStringArrayStorage) stringArrayStorage: IStringArrayStorage,
+        @inject(ServiceIdentifiers.IStringArrayStorage) stringArrayStorage: IStringArrayStorage,
         @inject(ServiceIdentifiers.Factory__IIdentifierNamesGenerator)
             identifierNamesGeneratorFactory: TIdentifierNamesGeneratorFactory,
         @inject(ServiceIdentifiers.IRandomGenerator) randomGenerator: IRandomGenerator,

src/node-transformers/obfuscating-transformers/obfuscating-replacers/literal-obfuscating-replacers/StringLiteralObfuscatingReplacer.ts

@@ -48,7 +48,7 @@ export class StringLiteralObfuscatingReplacer extends AbstractObfuscatingReplace
      * @param {IOptions} options
      */
     public constructor (
-        @inject(ServiceIdentifiers.TStringArrayStorage) stringArrayStorage: IStringArrayStorage,
+        @inject(ServiceIdentifiers.IStringArrayStorage) stringArrayStorage: IStringArrayStorage,
         @inject(ServiceIdentifiers.IStringArrayStorageAnalyzer) stringArrayStorageAnalyzer: IStringArrayStorageAnalyzer,
         @inject(ServiceIdentifiers.IOptions) options: IOptions
     ) {

src/storages/string-array/StringArrayStorage.ts

@@ -71,6 +71,11 @@ export class StringArrayStorage extends MapStorage <string, IStringArrayStorageI
      */
     private readonly rc4Keys: string[];
 
+    /**
+     * @type {Map<string, string[]>}
+     */
+    private readonly rc4EncodedValuesSourcesCache: Map<string, string[]> = new Map();
+
     /**
      * @type {number}
      */
@@ -256,10 +261,40 @@ export class StringArrayStorage extends MapStorage <string, IStringArrayStorageI
      */
     private getEncodedValue (value: string): IEncodedValue {
         switch (this.options.stringArrayEncoding) {
+            /**
+             * For rc4 there is a possible chance of a collision between encoded values that were received from
+             * different source values with different keys
+             *
+             * For example:
+             * source value | key  | encoded value
+             * _15          | CRDL | w74TGA==
+             * _12          | q9mB | w74TGA==
+             *
+             * Issue: https://github.com/javascript-obfuscator/javascript-obfuscator/issues/538
+             *
+             * As a fix that keeps key size of 4 character, the simple brute-force solution is using:
+             * if collision will happen, just try to encode value again
+             */
             case StringArrayEncoding.Rc4: {
                 const decodeKey: string = this.randomGenerator.getRandomGenerator().pickone(this.rc4Keys);
                 const encodedValue: string = this.cryptUtils.btoa(this.cryptUtils.rc4(value, decodeKey));
 
+                const encodedValueSources: string[] = this.rc4EncodedValuesSourcesCache.get(encodedValue) ?? [];
+                let encodedValueSourcesLength: number = encodedValueSources.length;
+
+                const shouldAddValueToSourcesCache: boolean = !encodedValueSourcesLength || !encodedValueSources.includes(value);
+
+                if (shouldAddValueToSourcesCache) {
+                    encodedValueSources.push(value);
+                    encodedValueSourcesLength++;
+                }
+
+                this.rc4EncodedValuesSourcesCache.set(encodedValue, encodedValueSources);
+
+                if (encodedValueSourcesLength > 1) {
+                    return this.getEncodedValue(value);
+                }
+
                 return { encodedValue, decodeKey };
             }
 

test/index.spec.ts

@@ -39,6 +39,7 @@ import './unit-tests/options/ValidationErrorsFormatter.spec';
 import './unit-tests/source-code/ObfuscatedCode.spec';
 import './unit-tests/storages/ArrayStorage.spec';
 import './unit-tests/storages/MapStorage.spec';
+import './unit-tests/storages/string-array/StringArrayStorage.spec';
 import './unit-tests/utils/ArrayUtils.spec';
 import './unit-tests/utils/CryptUtils.spec';
 import './unit-tests/utils/EscapeSequenceEncoder.spec';

test/unit-tests/storages/string-array/StringArrayStorage.spec.ts

@@ -0,0 +1,75 @@
+import 'reflect-metadata';
+
+import { assert } from 'chai';
+
+import { ServiceIdentifiers } from '../../../../src/container/ServiceIdentifiers';
+
+import { TInputOptions } from '../../../../src/types/options/TInputOptions';
+
+import { IInversifyContainerFacade } from '../../../../src/interfaces/container/IInversifyContainerFacade';
+import { IStringArrayStorage } from '../../../../src/interfaces/storages/string-array-storage/IStringArrayStorage';
+
+import { StringArrayEncoding } from '../../../../src/enums/StringArrayEncoding';
+
+import { NO_ADDITIONAL_NODES_PRESET } from '../../../../src/options/presets/NoCustomNodes';
+
+import { InversifyContainerFacade } from '../../../../src/container/InversifyContainerFacade';
+
+/**
+ * @returns {IMapStorage<string, V>}
+ */
+const getStorageInstance = (options: TInputOptions = {}): IStringArrayStorage => {
+    const inversifyContainerFacade: IInversifyContainerFacade = new InversifyContainerFacade();
+
+    inversifyContainerFacade.load('', '', {
+        ...NO_ADDITIONAL_NODES_PRESET,
+        stringArray: true,
+        stringArrayThreshold: 1,
+        ...options
+    });
+
+    const storage: IStringArrayStorage = inversifyContainerFacade.get(ServiceIdentifiers.IStringArrayStorage);
+
+    storage.initialize();
+
+    return storage;
+};
+
+const getEncodedValue = (stringArrayStorage: IStringArrayStorage, value: string, decodeKey: string): string => {
+    (<any>stringArrayStorage).rc4Keys = [
+        'foo',
+        decodeKey
+    ];
+
+    return stringArrayStorage.get(value)?.encodedValue ?? '';
+};
+
+describe('StringArrayStorage', () => {
+    describe('rc4 encoded value collision fix', () => {
+        const samplesCount: number = 100;
+
+        let firstEncodedValue: string;
+        let secondEncodedValue: string;
+        let isCollisionHappened: boolean = false;
+
+        before(() => {
+            const stringArrayStorage: IStringArrayStorage = getStorageInstance({
+                stringArrayEncoding: StringArrayEncoding.Rc4
+            });
+
+            for (let i = 0; i < samplesCount; i++) {
+                firstEncodedValue = getEncodedValue(stringArrayStorage, '_15', 'CRDL');
+                secondEncodedValue = getEncodedValue(stringArrayStorage, '_12', 'q9mB');
+
+                if (firstEncodedValue === secondEncodedValue) {
+                    isCollisionHappened = true;
+                    break;
+                }
+            }
+        });
+
+        it('should not make a collision between different source values with different keys', () => {
+            assert.equal(isCollisionHappened, false);
+        });
+    });
+});