From e0bdec96f9414db28e741baba7f68c224d9339f6 Mon Sep 17 00:00:00 2001 From: Adriano Cola Date: Mon, 23 Sep 2024 15:21:16 -0300 Subject: [PATCH] fix: secret key stored cleartext in memory --- .../onboarding/sign-in/hooks/use-sign-in.ts | 2 +- src/app/routes/account-gate.tsx | 10 ++--- src/app/routes/onboarding-gate.tsx | 15 +++++--- .../in-memory-key/in-memory-key.selectors.ts | 37 +++++++++++++++---- .../in-memory-key/in-memory-key.slice.ts | 12 +++--- src/app/store/session-restore.ts | 2 +- .../software-keys/software-key.actions.ts | 2 +- src/shared/utils/text-encoding.spec.ts | 25 +++++++++++++ src/shared/utils/text-encoding.ts | 20 ++++++++++ 9 files changed, 97 insertions(+), 28 deletions(-) create mode 100644 src/shared/utils/text-encoding.spec.ts create mode 100644 src/shared/utils/text-encoding.ts diff --git a/src/app/pages/onboarding/sign-in/hooks/use-sign-in.ts b/src/app/pages/onboarding/sign-in/hooks/use-sign-in.ts index 7f7dffbba2f..30ad9fcb671 100644 --- a/src/app/pages/onboarding/sign-in/hooks/use-sign-in.ts +++ b/src/app/pages/onboarding/sign-in/hooks/use-sign-in.ts @@ -58,7 +58,7 @@ export function useSignIn() { await simulateShortDelayToAvoidImmediateNavigation(); - dispatch(inMemoryKeyActions.saveUsersSecretKeyToBeRestored(parsedKeyInput)); + dispatch(inMemoryKeyActions.setDefaultKey(parsedKeyInput)); void analytics.track('submit_valid_secret_key'); navigate(RouteUrls.SetPassword); setIsIdle(); diff --git a/src/app/routes/account-gate.tsx b/src/app/routes/account-gate.tsx index 6f14cf23c46..ecfad5e1576 100644 --- a/src/app/routes/account-gate.tsx +++ b/src/app/routes/account-gate.tsx @@ -3,7 +3,7 @@ import { Navigate } from 'react-router-dom'; import { RouteUrls } from '@shared/route-urls'; -import { useDefaultWalletSecretKey } from '@app/store/in-memory-key/in-memory-key.selectors'; +import { useHasDefaultInMemoryWalletSecretKey } from '@app/store/in-memory-key/in-memory-key.selectors'; import { useHasLedgerKeys } from '@app/store/ledger/ledger.selectors'; import { useCurrentKeyDetails } from '@app/store/software-keys/software-key.selectors'; @@ -11,8 +11,8 @@ export function shouldNavigateToOnboardingStartPage(currentKeyDetails?: any) { return !currentKeyDetails; } -export function shouldNavigateToUnlockWalletPage(currentInMemorySecretKey?: string) { - return !currentInMemorySecretKey; +export function shouldNavigateToUnlockWalletPage(hasDefaultInMemorySecretKey: boolean) { + return !hasDefaultInMemorySecretKey; } interface AccountGateProps { @@ -20,7 +20,7 @@ interface AccountGateProps { } export function AccountGate({ children }: AccountGateProps) { const currentKeyDetails = useCurrentKeyDetails(); - const currentInMemorySecretKey = useDefaultWalletSecretKey(); + const hasDefaultInMemorySecretKey = useHasDefaultInMemoryWalletSecretKey(); const isLedger = useHasLedgerKeys(); if (isLedger) return <>{children}; @@ -28,7 +28,7 @@ export function AccountGate({ children }: AccountGateProps) { if (shouldNavigateToOnboardingStartPage(currentKeyDetails)) return ; - if (shouldNavigateToUnlockWalletPage(currentInMemorySecretKey)) + if (shouldNavigateToUnlockWalletPage(hasDefaultInMemorySecretKey)) return ; return <>{children}; diff --git a/src/app/routes/onboarding-gate.tsx b/src/app/routes/onboarding-gate.tsx index 9ba0efb67ea..27ebecc92ef 100644 --- a/src/app/routes/onboarding-gate.tsx +++ b/src/app/routes/onboarding-gate.tsx @@ -3,12 +3,15 @@ import { Navigate } from 'react-router-dom'; import { RouteUrls } from '@shared/route-urls'; -import { useDefaultWalletSecretKey } from '@app/store/in-memory-key/in-memory-key.selectors'; +import { useHasDefaultInMemoryWalletSecretKey } from '@app/store/in-memory-key/in-memory-key.selectors'; import { useHasLedgerKeys } from '@app/store/ledger/ledger.selectors'; import { useCurrentKeyDetails } from '@app/store/software-keys/software-key.selectors'; -function hasAlreadyMadeWalletAndPlaintextKeyInMemory(encryptedKey?: string, inMemoryKey?: string) { - return !!encryptedKey && !!inMemoryKey; +function hasAlreadyMadeWalletAndPlaintextKeyInMemory( + hasInMemorySecretKey: boolean, + encryptedKey?: string +) { + return hasInMemorySecretKey && !!encryptedKey; } function keyDetailsExistsWalletAlreadyCreatedSoPreventOnboarding(keyDetails: unknown) { @@ -20,14 +23,14 @@ interface OnboardingGateProps { } export function OnboardingGate({ children }: OnboardingGateProps) { const keyDetails = useCurrentKeyDetails(); - const currentInMemoryKey = useDefaultWalletSecretKey(); + const hasInMemorySecretKey = useHasDefaultInMemoryWalletSecretKey(); const isLedger = useHasLedgerKeys(); if ( (keyDetails?.type === 'software' && hasAlreadyMadeWalletAndPlaintextKeyInMemory( - keyDetails.encryptedSecretKey, - currentInMemoryKey + hasInMemorySecretKey, + keyDetails.encryptedSecretKey )) || isLedger ) { diff --git a/src/app/store/in-memory-key/in-memory-key.selectors.ts b/src/app/store/in-memory-key/in-memory-key.selectors.ts index 0a9ef5ee5f2..059d4dbf447 100644 --- a/src/app/store/in-memory-key/in-memory-key.selectors.ts +++ b/src/app/store/in-memory-key/in-memory-key.selectors.ts @@ -3,22 +3,45 @@ import { useSelector } from 'react-redux'; import { createSelector } from '@reduxjs/toolkit'; import { defaultWalletKeyId } from '@shared/utils'; +import { decodeText } from '@shared/utils/text-encoding'; import { mnemonicToRootNode } from '@app/common/keychain/keychain'; import { RootState } from '..'; -const selectInMemoryKey = (state: RootState) => state.inMemoryKeys; +const selectInMemoryKeys = (state: RootState) => state.inMemoryKeys; -export const selectDefaultWalletKey = createSelector( - selectInMemoryKey, +const selectDefaultInMemoryWalletKeyBytes = createSelector( + selectInMemoryKeys, state => state.keys[defaultWalletKeyId] ); -export const selectRootKeychain = createSelector(selectDefaultWalletKey, key => { - if (!key) return null; - return mnemonicToRootNode(key); -}); +const selectHasDefaultInMemoryWalletKey = createSelector( + selectDefaultInMemoryWalletKeyBytes, + key => !!key +); + +export function useHasDefaultInMemoryWalletSecretKey() { + return useSelector(selectHasDefaultInMemoryWalletKey); +} + +// Not using a memoized "createSelector" to avoid storing the decoded key as cleartext in memory +export const selectDefaultWalletKey = (state: RootState) => { + const defaultWalletBytes = selectDefaultInMemoryWalletKeyBytes(state); + + if (!defaultWalletBytes) return null; + + return decodeText(defaultWalletBytes); +}; + +export const selectRootKeychain = createSelector( + selectDefaultInMemoryWalletKeyBytes, + defaultKey => { + if (!defaultKey) return null; + + return mnemonicToRootNode(decodeText(defaultKey)); + } +); export function useDefaultWalletSecretKey() { return useSelector(selectDefaultWalletKey); diff --git a/src/app/store/in-memory-key/in-memory-key.slice.ts b/src/app/store/in-memory-key/in-memory-key.slice.ts index 80610cc29f5..f970f86ab13 100644 --- a/src/app/store/in-memory-key/in-memory-key.slice.ts +++ b/src/app/store/in-memory-key/in-memory-key.slice.ts @@ -2,12 +2,13 @@ import { PayloadAction, createSlice } from '@reduxjs/toolkit'; import { logger } from '@shared/logger'; import { defaultWalletKeyId } from '@shared/utils'; +import { encodeText } from '@shared/utils/text-encoding'; import { keySlice } from '../software-keys/software-key.slice'; interface InMemoryKeyState { hasRestoredKeys: boolean; - keys: Record; + keys: Record; } const initialState: InMemoryKeyState = { @@ -25,15 +26,12 @@ export const inMemoryKeySlice = createSlice({ logger.warn('Not generating another wallet, already exists.'); return; } - state.keys[defaultWalletKeyId] = action.payload; - }, - saveUsersSecretKeyToBeRestored(state, action: PayloadAction) { - state.keys[defaultWalletKeyId] = action.payload; + state.keys[defaultWalletKeyId] = encodeText(action.payload); }, - setKeysInMemory(state, action: PayloadAction>) { - return { ...state, hasRestoredKeys: true, keys: { ...state.keys, ...action.payload } }; + setDefaultKey(state, action: PayloadAction) { + state.keys[defaultWalletKeyId] = encodeText(action.payload); }, lockWallet(state) { diff --git a/src/app/store/session-restore.ts b/src/app/store/session-restore.ts index c32bbf5119b..9ef3a25237b 100644 --- a/src/app/store/session-restore.ts +++ b/src/app/store/session-restore.ts @@ -24,7 +24,7 @@ export async function restoreWalletSession() { if (currentKey?.type === 'software') { const secretKey = await decrypt(currentKey.encryptedSecretKey, key.encryptionKey); - store.dispatch(inMemoryKeyActions.setKeysInMemory({ default: secretKey })); + store.dispatch(inMemoryKeyActions.setDefaultKey(secretKey)); } } catch (e) { logger.error('Failed to decrypt secret key'); diff --git a/src/app/store/software-keys/software-key.actions.ts b/src/app/store/software-keys/software-key.actions.ts index d6ad4bccd45..2ae213c6fcf 100644 --- a/src/app/store/software-keys/software-key.actions.ts +++ b/src/app/store/software-keys/software-key.actions.ts @@ -126,7 +126,7 @@ function unlockWalletAction(password: string): AppThunk { if (!rootKey.publicKey) throw new Error('Could not derive root key from mnemonic'); void identifyUser(rootKey.publicKey); - dispatch(inMemoryKeySlice.actions.setKeysInMemory({ default: secretKey })); + dispatch(inMemoryKeySlice.actions.setDefaultKey(secretKey)); }; } diff --git a/src/shared/utils/text-encoding.spec.ts b/src/shared/utils/text-encoding.spec.ts new file mode 100644 index 00000000000..2eb1887267f --- /dev/null +++ b/src/shared/utils/text-encoding.spec.ts @@ -0,0 +1,25 @@ +import { decodeText, encodeText } from './text-encoding'; + +describe('encode and decode text', () => { + it('works for UTF-8 text', () => { + const text = 'a Ā 𐀀 文 ❤️'; + const encoded = encodeText(text); + const decoded = decodeText(encoded); + + expect(decoded).toEqual(text); + }); + + it('works for empty strings', () => { + const text = ''; + const encoded = encodeText(text); + const decoded = decodeText(encoded); + expect(decoded).toEqual(text); + }); + + it('does not simply convert to ASCII codes', () => { + const text = 'a'; + const textAsciiHex = '61'; + const encoded = encodeText(text); + expect(encoded).not.toEqual(textAsciiHex); + }); +}); diff --git a/src/shared/utils/text-encoding.ts b/src/shared/utils/text-encoding.ts new file mode 100644 index 00000000000..8ecbbe70d52 --- /dev/null +++ b/src/shared/utils/text-encoding.ts @@ -0,0 +1,20 @@ +/** + * Encode `text` to a hex string. Accepts text with any characters (like emojis). + * Does not match ASCII codes for latin-script letters (ie: "a" will not be encoded to "61"). + * @param text string to encode + */ +export function encodeText(text: string) { + return Array.from(new TextEncoder().encode(btoa(encodeURIComponent(text)))) + .map(byte => byte.toString(16).padStart(2, '0')) + .join(''); +} + +/** + * Decode `hex` previously encoded with [encodeText]{@link encodeText} to the original text. + * @param hex string to decode + */ +export function decodeText(hex?: string) { + if (!hex) return ''; + const bytes = new Uint8Array((hex.match(/.{1,2}/g) ?? []).map(byte => parseInt(byte, 16))); + return decodeURIComponent(atob(new TextDecoder().decode(bytes))); +}