Skip to content

Commit

Permalink
fix: secret key stored cleartext in memory
Browse files Browse the repository at this point in the history
  • Loading branch information
adrianocola authored and kyranjamie committed Oct 1, 2024
1 parent 9c0a6e8 commit e0bdec9
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/app/pages/onboarding/sign-in/hooks/use-sign-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 5 additions & 5 deletions src/app/routes/account-gate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,32 @@ 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';

export function shouldNavigateToOnboardingStartPage(currentKeyDetails?: any) {
return !currentKeyDetails;
}

export function shouldNavigateToUnlockWalletPage(currentInMemorySecretKey?: string) {
return !currentInMemorySecretKey;
export function shouldNavigateToUnlockWalletPage(hasDefaultInMemorySecretKey: boolean) {
return !hasDefaultInMemorySecretKey;
}

interface AccountGateProps {
children?: ReactNode;
}
export function AccountGate({ children }: AccountGateProps) {
const currentKeyDetails = useCurrentKeyDetails();
const currentInMemorySecretKey = useDefaultWalletSecretKey();
const hasDefaultInMemorySecretKey = useHasDefaultInMemoryWalletSecretKey();

const isLedger = useHasLedgerKeys();
if (isLedger) return <>{children}</>;

if (shouldNavigateToOnboardingStartPage(currentKeyDetails))
return <Navigate to={RouteUrls.Onboarding} />;

if (shouldNavigateToUnlockWalletPage(currentInMemorySecretKey))
if (shouldNavigateToUnlockWalletPage(hasDefaultInMemorySecretKey))
return <Navigate to={RouteUrls.Unlock} />;

return <>{children}</>;
Expand Down
15 changes: 9 additions & 6 deletions src/app/routes/onboarding-gate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
) {
Expand Down
37 changes: 30 additions & 7 deletions src/app/store/in-memory-key/in-memory-key.selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 5 additions & 7 deletions src/app/store/in-memory-key/in-memory-key.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string | undefined>;
keys: Record<string, string>;
}

const initialState: InMemoryKeyState = {
Expand All @@ -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<string>) {
state.keys[defaultWalletKeyId] = action.payload;
state.keys[defaultWalletKeyId] = encodeText(action.payload);
},

setKeysInMemory(state, action: PayloadAction<Record<string, string>>) {
return { ...state, hasRestoredKeys: true, keys: { ...state.keys, ...action.payload } };
setDefaultKey(state, action: PayloadAction<string>) {
state.keys[defaultWalletKeyId] = encodeText(action.payload);
},

lockWallet(state) {
Expand Down
2 changes: 1 addition & 1 deletion src/app/store/session-restore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion src/app/store/software-keys/software-key.actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
};
}

Expand Down
25 changes: 25 additions & 0 deletions src/shared/utils/text-encoding.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
20 changes: 20 additions & 0 deletions src/shared/utils/text-encoding.ts
Original file line number Diff line number Diff line change
@@ -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)));
}

0 comments on commit e0bdec9

Please sign in to comment.