From d182bfded9b2df225aedc99ce5c640eb523b9597 Mon Sep 17 00:00:00 2001 From: lukaw3d Date: Sat, 26 Aug 2023 00:51:32 +0200 Subject: [PATCH] Before transaction validate if ledger device still derives same account This improves the error message for users mixing up multiple ledger devices. --- src/app/components/ErrorFormatter/index.tsx | 4 ++++ src/app/lib/__tests__/ledger.test.ts | 19 ++++++++++++------- src/app/lib/ledger.ts | 21 ++++++++++++++++++++- src/locales/en/translation.json | 1 + src/types/errors.ts | 1 + 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/app/components/ErrorFormatter/index.tsx b/src/app/components/ErrorFormatter/index.tsx index a7a68854e7..949f1f648c 100644 --- a/src/app/components/ErrorFormatter/index.tsx +++ b/src/app/components/ErrorFormatter/index.tsx @@ -76,6 +76,10 @@ export function ErrorFormatter(props: Props) { 'errors.LedgerOasisAppIsNotOpen', 'Oasis App on Ledger is closed.', ), + [WalletErrors.LedgerDerivedDifferentAccount]: t( + 'errors.LedgerDerivedDifferentAccount', + 'This account does not belong to the currently connected Ledger.', + ), [WalletErrors.LedgerUnknownError]: t('errors.unknownLedgerError', 'Unknown ledger error: {{message}}', { message, }), diff --git a/src/app/lib/__tests__/ledger.test.ts b/src/app/lib/__tests__/ledger.test.ts index 6335dd148b..19068cd06c 100644 --- a/src/app/lib/__tests__/ledger.test.ts +++ b/src/app/lib/__tests__/ledger.test.ts @@ -139,13 +139,6 @@ describe('Ledger Library', () => { }) it('Should return the public key', () => { - const sign = jest.mocked(OasisApp.prototype.sign) - sign.mockResolvedValueOnce({ - return_code: 0x9000, - signature: Buffer.from(new Uint8Array([1, 2, 3])), - error_message: '', - }) - const signer = new LedgerSigner({ type: WalletType.Ledger, path: [44, 474, 0, 0, 0], @@ -157,6 +150,12 @@ describe('Ledger Library', () => { }) it('Should throw if the transaction was rejected', async () => { + const pubKey = jest.mocked(OasisApp.prototype.publicKey) + pubKey.mockResolvedValueOnce({ + return_code: 0x9000, + pk: Buffer.from(new Uint8Array([0])), + error_message: '', + }) const sign = jest.mocked(OasisApp.prototype.sign) sign.mockResolvedValueOnce({ return_code: 0x6986, error_message: '' }) @@ -174,6 +173,12 @@ describe('Ledger Library', () => { }) it('Should return the signature', async () => { + const pubKey = jest.mocked(OasisApp.prototype.publicKey) + pubKey.mockResolvedValueOnce({ + return_code: 0x9000, + pk: Buffer.from(new Uint8Array([0])), + error_message: '', + }) const sign = jest.mocked(OasisApp.prototype.sign) sign.mockResolvedValueOnce({ return_code: 0x9000, diff --git a/src/app/lib/ledger.ts b/src/app/lib/ledger.ts index 44d87c1bbc..c6df15f955 100644 --- a/src/app/lib/ledger.ts +++ b/src/app/lib/ledger.ts @@ -3,7 +3,7 @@ import OasisApp, { successOrThrow } from '@oasisprotocol/ledger' import { Response } from '@oasisprotocol/ledger/dist/types' import { Wallet, WalletType } from 'app/state/wallet/types' import { WalletError, WalletErrors } from 'types/errors' -import { hex2uint } from './helpers' +import { hex2uint, publicKeyToAddress } from './helpers' import type Transport from '@ledgerhq/hw-transport' import { isSupported, requestLedgerDevice } from '@ledgerhq/hw-transport-webusb/lib-es/webusb' @@ -74,6 +74,24 @@ export class Ledger { } return accounts } + + public static async validateAccountDerivation( + app: OasisApp, + path: number[], + expectedPublicKey: Uint8Array, + ) { + const expectedAddress = await publicKeyToAddress(expectedPublicKey) + + const publicKeyResponse = successOrThrowWalletError(await app.publicKey(path), 'ledger public key') + const publicKey = new Uint8Array(publicKeyResponse.pk) + const reDerivedAddress = await publicKeyToAddress(publicKey) + if (expectedAddress !== reDerivedAddress) { + throw new WalletError( + WalletErrors.LedgerDerivedDifferentAccount, + `expected to derive ${expectedAddress} but ledger derived ${reDerivedAddress}`, + ) + } + } } export class LedgerSigner implements ContextSigner { @@ -104,6 +122,7 @@ export class LedgerSigner implements ContextSigner { } const app = new OasisApp(this.transport) + await Ledger.validateAccountDerivation(app, this.path, this.publicKey) const response = successOrThrowWalletError( await app.sign(this.path, context, Buffer.from(message)), 'ledger sign', diff --git a/src/locales/en/translation.json b/src/locales/en/translation.json index 3704eb0388..d14d79db32 100644 --- a/src/locales/en/translation.json +++ b/src/locales/en/translation.json @@ -135,6 +135,7 @@ "reclaimedAmount": "Amount to reclaim" }, "errors": { + "LedgerDerivedDifferentAccount": "This account does not belong to the currently connected Ledger.", "LedgerOasisAppIsNotOpen": "Oasis App on Ledger is closed.", "cannotSendToSelf": "Cannot send to yourself", "disconnectedError": "Lost connection.", diff --git a/src/types/errors.ts b/src/types/errors.ts index 5bc453cbd1..7deddb4a58 100644 --- a/src/types/errors.ts +++ b/src/types/errors.ts @@ -26,6 +26,7 @@ export enum WalletErrors { LedgerNoDeviceSelected = 'no_device_selected', LedgerTransactionRejected = 'transaction_rejected', LedgerAppVersionNotSupported = 'ledger_version_not_supported', + LedgerDerivedDifferentAccount = 'ledger_derived_different_account', IndexerAPIError = 'indexer_api_error', DisconnectedError = 'disconnected_error', ParaTimesUnknownError = 'para_times_unknown_error',