Skip to content

Commit

Permalink
Merge pull request #1614 from oasisprotocol/lw/wrong-ledger
Browse files Browse the repository at this point in the history
Before transaction validate if ledger device still derives same account
  • Loading branch information
lukaw3d authored Aug 28, 2023
2 parents 18faf5c + d182bfd commit 30c8e6d
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 8 deletions.
4 changes: 4 additions & 0 deletions src/app/components/ErrorFormatter/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
Expand Down
19 changes: 12 additions & 7 deletions src/app/lib/__tests__/ledger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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: '' })

Expand All @@ -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,
Expand Down
21 changes: 20 additions & 1 deletion src/app/lib/ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions src/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
1 change: 1 addition & 0 deletions src/types/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 30c8e6d

Please sign in to comment.