Encryption: Fix - reject plaintext in protected fields

Author: gabrielbazan7

src/store/transforms/encrypt.spec.ts

@@ -1,11 +1,15 @@
 import crypto from 'crypto';
 import Aes from 'crypto-js/aes.js';
 import {
+  decryptAppStore,
   decryptPersistValue,
   deserializePersistValue,
+  decryptShopStore,
   decryptValue,
   decryptWalletStore,
+  encryptAppStore,
   encryptPersistValue,
+  encryptShopStore,
   encryptValue,
   encryptWalletStore,
 } from './encrypt';
@@ -67,6 +71,130 @@ describe('encrypted field values', () => {
     };
     expect(() => decryptWalletStore(swapped, secretKey)).toThrow();
   });
+
+  type ProtectedStoreCase = {
+    name: string;
+    fields: string[];
+    buildState: (field: string, value: unknown) => any;
+    encrypt: (state: any) => any;
+    decrypt: (state: any) => any;
+    read: (state: any, field: string) => unknown;
+    readPublic: (state: any) => unknown;
+  };
+
+  const protectedStoreCases: ProtectedStoreCase[] = [
+    {
+      name: 'WALLET',
+      fields: [
+        'mnemonic',
+        'mnemonicEncrypted',
+        'xPrivKey',
+        'xPrivKeyEncrypted',
+        'xPrivKeyEDDSA',
+        'xPrivKeyEDDSAEncrypted',
+      ],
+      buildState: (field, value) => ({
+        keys: {
+          keyA: {properties: {[field]: value, fingerPrint: 'public-value'}},
+        },
+      }),
+      encrypt: state => encryptWalletStore(state, secretKey),
+      decrypt: state => decryptWalletStore(state, secretKey),
+      read: (state, field) => state.keys.keyA.properties[field],
+      readPublic: state => state.keys.keyA.properties.fingerPrint,
+    },
+    {
+      name: 'APP',
+      fields: ['priv'],
+      buildState: (field, value) => ({
+        identity: {livenet: {[field]: value, pub: 'public-value'}},
+      }),
+      encrypt: state => encryptAppStore(state, secretKey),
+      decrypt: state => decryptAppStore(state, secretKey),
+      read: (state, field) => state.identity.livenet[field],
+      readPublic: state => state.identity.livenet.pub,
+    },
+    {
+      name: 'SHOP',
+      fields: [
+        'accessKey',
+        'barcodeData',
+        'barcodeImage',
+        'claimCode',
+        'claimLink',
+        'pin',
+      ],
+      buildState: (field, value) => ({
+        giftCards: {
+          livenet: [{[field]: value, displayName: 'public-value'}],
+        },
+      }),
+      encrypt: state => encryptShopStore(state, secretKey),
+      decrypt: state => decryptShopStore(state, secretKey),
+      read: (state, field) => state.giftCards.livenet[0][field],
+      readPublic: state => state.giftCards.livenet[0].displayName,
+    },
+  ];
+
+  describe.each(protectedStoreCases)('$name protected fields', storeCase => {
+    const firstField = storeCase.fields[0];
+
+    it.each(storeCase.fields)('rejects plaintext in %s', field => {
+      expect(() =>
+        storeCase.decrypt(storeCase.buildState(field, 'attacker-controlled')),
+      ).toThrow(field);
+    });
+
+    it('rejects a non-string value', () => {
+      expect(() =>
+        storeCase.decrypt(storeCase.buildState(firstField, {injected: true})),
+      ).toThrow('Expected encrypted protected value');
+    });
+
+    it.each(storeCase.fields)(
+      'accepts legacy CBC and modern GCM in %s without changing public fields',
+      field => {
+        const plaintext = `${storeCase.name}-secret`;
+        const legacy = `encrypted:${Aes.encrypt(
+          plaintext,
+          secretKey,
+        ).toString()}`;
+        const legacyState = storeCase.decrypt(
+          storeCase.buildState(field, legacy),
+        );
+        expect(storeCase.read(legacyState, field)).toBe(plaintext);
+
+        const modernState = storeCase.encrypt(
+          storeCase.buildState(field, plaintext),
+        );
+        expect(storeCase.read(modernState, field)).toMatch(/^field-aesgcm-v1:/);
+        const decrypted = storeCase.decrypt(modernState);
+        expect(storeCase.read(decrypted, field)).toBe(plaintext);
+        expect(storeCase.readPublic(decrypted)).toBe('public-value');
+      },
+    );
+
+    it.each([undefined, null, ''])('allows an absent value (%p)', value => {
+      const state = storeCase.buildState(firstField, value);
+      expect(storeCase.read(storeCase.decrypt(state), firstField)).toBe(value);
+    });
+  });
+
+  it('does not include rejected plaintext in the error', () => {
+    const plaintext = 'attacker-controlled-secret';
+    let capturedError: Error | undefined;
+
+    try {
+      protectedStoreCases[0].decrypt(
+        protectedStoreCases[0].buildState('mnemonic', plaintext),
+      );
+    } catch (err) {
+      capturedError = err as Error;
+    }
+
+    expect(capturedError).toBeDefined();
+    expect(capturedError!.message).not.toContain(plaintext);
+  });
 });
 
 describe('persisted reducer values', () => {

src/store/transforms/encrypt.ts

@@ -21,6 +21,9 @@ const isLegacyEncryptedValue = (value: string) =>
 const isEncryptedValue = (value: string) =>
   isModernEncryptedValue(value) || isLegacyEncryptedValue(value);
 
+const hasProtectedValue = (value: unknown): boolean =>
+  value !== undefined && value !== null && value !== '';
+
 const buildAesKey = (secretKey: string): Buffer => {
   return crypto.createHash('sha256').update(secretKey).digest();
 };
@@ -155,10 +158,14 @@ export const decryptValue = (
   secretKey: string,
   context = defaultFieldContext,
 ): any => {
-  if (typeof value !== 'string' || !isEncryptedValue(value)) {
+  if (!hasProtectedValue(value)) {
     return value;
   }
 
+  if (typeof value !== 'string' || !isEncryptedValue(value)) {
+    throw new Error(`Expected encrypted protected value at ${context}`);
+  }
+
   if (value.startsWith(modernEncryptedPrefix)) {
     return decryptWithAesGcm(value, secretKey, modernEncryptedPrefix, context);
   }
@@ -224,7 +231,7 @@ const transformWalletStore = (
   state: any,
   secretKey: string,
   transformer: (value: any, secretKey: string, context: string) => any,
-  checkCondition: (value: string) => boolean,
+  checkCondition: (value: any) => boolean,
 ): any => {
   if (!state || !state.keys) {
     return state;
@@ -250,7 +257,7 @@ const transformWalletStore = (
     const updatedProperties = fieldsToTransform.reduce(
       (latestProperties, field) => {
         const value = properties[field];
-        if (value && typeof value === 'string' && checkCondition(value)) {
+        if (hasProtectedValue(value) && checkCondition(value)) {
           latestProperties[field] = transformer(
             value,
             secretKey,
@@ -278,34 +285,32 @@ export const encryptWalletStore = (state: any, secretKey: string): any => {
     state,
     secretKey,
     encryptValue,
-    value => !isEncryptedValue(value),
+    value => typeof value === 'string' && !isEncryptedValue(value),
   );
 };
 
 export const decryptWalletStore = (state: any, secretKey: string): any => {
-  return transformWalletStore(state, secretKey, decryptValue, value =>
-    isEncryptedValue(value),
-  );
+  return transformWalletStore(state, secretKey, decryptValue, () => true);
 };
 
 // Generic function to transform app store (encrypt or decrypt)
 const transformAppStore = (
   state: any,
   secretKey: string,
   transformer: (value: any, secretKey: string, context: string) => any,
-  checkCondition: (value: string) => boolean,
+  checkCondition: (value: any) => boolean,
 ): any => {
   if (!state || !state.identity) {
     return state;
   }
 
   const identity = state.identity[Network.mainnet];
-  if (!identity || !identity.priv) {
+  if (!identity) {
     return state;
   }
 
   const privValue = identity.priv;
-  if (privValue && typeof privValue === 'string' && checkCondition(privValue)) {
+  if (hasProtectedValue(privValue) && checkCondition(privValue)) {
     return {
       ...state,
       identity: {
@@ -329,22 +334,20 @@ export const encryptAppStore = (state: any, secretKey: string): any => {
     state,
     secretKey,
     encryptValue,
-    value => !isEncryptedValue(value),
+    value => typeof value === 'string' && !isEncryptedValue(value),
   );
 };
 
 export const decryptAppStore = (state: any, secretKey: string): any => {
-  return transformAppStore(state, secretKey, decryptValue, value =>
-    isEncryptedValue(value),
-  );
+  return transformAppStore(state, secretKey, decryptValue, () => true);
 };
 
 // Generic function to transform shop store (encrypt or decrypt)
 const transformShopStore = (
   state: any,
   secretKey: string,
   transformer: (value: any, secretKey: string, context: string) => any,
-  checkCondition: (value: string) => boolean,
+  checkCondition: (value: any) => boolean,
 ): any => {
   if (!state || !state.giftCards || !state.giftCards[Network.mainnet]) {
     return state;
@@ -369,7 +372,7 @@ const transformShopStore = (
     const updatedCard = {...card};
     fieldsToTransform.forEach(field => {
       const value = card[field];
-      if (value && typeof value === 'string' && checkCondition(value)) {
+      if (hasProtectedValue(value) && checkCondition(value)) {
         updatedCard[field] = transformer(
           value,
           secretKey,
@@ -396,12 +399,10 @@ export const encryptShopStore = (state: any, secretKey: string): any => {
     state,
     secretKey,
     encryptValue,
-    value => !isEncryptedValue(value),
+    value => typeof value === 'string' && !isEncryptedValue(value),
   );
 };
 
 export const decryptShopStore = (state: any, secretKey: string): any => {
-  return transformShopStore(state, secretKey, decryptValue, value =>
-    isEncryptedValue(value),
-  );
+  return transformShopStore(state, secretKey, decryptValue, () => true);
 };