diff --git a/packages/perspective/plan/get-address-view.test.ts b/packages/perspective/plan/get-address-view.test.ts index 78bc62deb8..677d6dd2da 100644 --- a/packages/perspective/plan/get-address-view.test.ts +++ b/packages/perspective/plan/get-address-view.test.ts @@ -7,10 +7,10 @@ import { } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; import { bech32ToAddress } from '@penumbra-zone/bech32/src/address'; -const mockIsControlledAddress = vi.hoisted(() => vi.fn()); +const mockGetAddressIndexByAddress = vi.hoisted(() => vi.fn()); vi.mock('@penumbra-zone/wasm/src/address', () => ({ - isControlledAddress: mockIsControlledAddress, + getAddressIndexByAddress: mockGetAddressIndexByAddress, })); describe('getAddressView()', () => { @@ -19,12 +19,12 @@ describe('getAddressView()', () => { const address = new Address({ inner: bech32ToAddress(addressAsBech32) }); beforeEach(() => { - mockIsControlledAddress.mockReset(); + mockGetAddressIndexByAddress.mockReset(); }); describe('when the address is controlled by the user represented by the full viewing key', () => { beforeEach(() => { - mockIsControlledAddress.mockImplementation(() => new AddressIndex({ account: 123 })); + mockGetAddressIndexByAddress.mockImplementation(() => new AddressIndex({ account: 123 })); }); test('returns a visible `AddressView`', () => { @@ -46,7 +46,7 @@ describe('getAddressView()', () => { describe('when the address is not controlled by the user represented by the full viewing key', () => { beforeEach(() => { - mockIsControlledAddress.mockImplementation(() => undefined); + mockGetAddressIndexByAddress.mockImplementation(() => undefined); }); test('returns an opaque `AddressView`', () => { diff --git a/packages/perspective/plan/get-address-view.ts b/packages/perspective/plan/get-address-view.ts index 36bb5998e8..d3a28f6fa8 100644 --- a/packages/perspective/plan/get-address-view.ts +++ b/packages/perspective/plan/get-address-view.ts @@ -2,11 +2,10 @@ import { Address, AddressView, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; -import { isControlledAddress } from '@penumbra-zone/wasm/src/address'; -import { bech32Address } from '@penumbra-zone/bech32/src/address'; +import { getAddressIndexByAddress } from '@penumbra-zone/wasm/src/address'; export const getAddressView = (address: Address, fullViewingKey: string): AddressView => { - const index = isControlledAddress(fullViewingKey, bech32Address(address)); + const index = getAddressIndexByAddress(fullViewingKey, address); if (index) { return new AddressView({ diff --git a/packages/router/src/grpc/custody/authorize/assert-swap-claim-addresses-belong-to-current-user.test.ts b/packages/router/src/grpc/custody/authorize/assert-swap-claim-addresses-belong-to-current-user.test.ts new file mode 100644 index 0000000000..52b6f39613 --- /dev/null +++ b/packages/router/src/grpc/custody/authorize/assert-swap-claim-addresses-belong-to-current-user.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from 'vitest'; +import { assertSwapClaimAddressesBelongToCurrentUser } from './assert-swap-claim-addresses-belong-to-current-user'; +import { + ActionPlan, + TransactionPlan, +} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; +import { Code, ConnectError } from '@connectrpc/connect'; +import { Address } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; + +const currentUserAddress1 = new Address({ + inner: new Uint8Array([1, 2, 3]), +}); + +const currentUserAddress2 = new Address({ + inner: new Uint8Array([4, 5, 6]), +}); + +const otherUserAddress = new Address({ + inner: new Uint8Array([7, 8, 9]), +}); + +const swapWithCurrentUserAddress1 = new ActionPlan({ + action: { + case: 'swap', + value: { + swapPlaintext: { + claimAddress: currentUserAddress1, + }, + }, + }, +}); + +const swapWithCurrentUserAddress2 = new ActionPlan({ + action: { + case: 'swap', + value: { + swapPlaintext: { + claimAddress: currentUserAddress2, + }, + }, + }, +}); + +const swapWithOtherUserAddress = new ActionPlan({ + action: { + case: 'swap', + value: { + swapPlaintext: { + claimAddress: otherUserAddress, + }, + }, + }, +}); + +const swapWithUndefinedAddress = new ActionPlan({ + action: { + case: 'swap', + value: { + swapPlaintext: {}, + }, + }, +}); + +const mockIsControlledAddress = (address?: Address) => + !!address && [currentUserAddress1, currentUserAddress2].includes(address); + +describe('assertSwapClaimAddressesBelongToCurrentUser()', () => { + describe('when the transaction plan has no swaps', () => { + it('does not throw', () => { + expect(() => + assertSwapClaimAddressesBelongToCurrentUser(new TransactionPlan(), mockIsControlledAddress), + ).not.toThrow(); + }); + }); + + describe('when the transaction plan has swaps', () => { + describe("when all of the swaps' `claimAddress`es belong to the current user", () => { + it('does not throw', () => { + const plan = new TransactionPlan({ + actions: [swapWithCurrentUserAddress1, swapWithCurrentUserAddress2], + }); + + expect(() => + assertSwapClaimAddressesBelongToCurrentUser(plan, mockIsControlledAddress), + ).not.toThrow(); + }); + }); + + describe("when any of the swaps' `claimAddress`es do not belong to the current user", () => { + it('throws a `ConnectError` with the `PermissionDenied` code', () => { + const plan = new TransactionPlan({ + actions: [swapWithCurrentUserAddress1, swapWithOtherUserAddress], + }); + + expect.assertions(2); + + try { + assertSwapClaimAddressesBelongToCurrentUser(plan, mockIsControlledAddress); + } catch (error) { + expect(error).toBeInstanceOf(ConnectError); + expect((error as ConnectError).code).toBe(Code.PermissionDenied); + } + }); + }); + + describe("when any of the swaps' `claimAddress`es are empty", () => { + it('throws a `ConnectError` with the `PermissionDenied` code', () => { + const plan = new TransactionPlan({ + actions: [swapWithUndefinedAddress], + }); + + expect.assertions(2); + + try { + assertSwapClaimAddressesBelongToCurrentUser(plan, mockIsControlledAddress); + } catch (error) { + expect(error).toBeInstanceOf(ConnectError); + expect((error as ConnectError).code).toBe(Code.PermissionDenied); + } + }); + }); + }); +}); diff --git a/packages/router/src/grpc/custody/authorize/assert-swap-claim-addresses-belong-to-current-user.ts b/packages/router/src/grpc/custody/authorize/assert-swap-claim-addresses-belong-to-current-user.ts new file mode 100644 index 0000000000..03f305f381 --- /dev/null +++ b/packages/router/src/grpc/custody/authorize/assert-swap-claim-addresses-belong-to-current-user.ts @@ -0,0 +1,19 @@ +import { Address } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; +import { TransactionPlan } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; +import { Code, ConnectError } from '@connectrpc/connect'; + +export const assertSwapClaimAddressesBelongToCurrentUser = ( + plan: TransactionPlan, + isControlledAddress: (address?: Address) => boolean, +): void => { + plan.actions.forEach(action => { + if (action.action.case !== 'swap') return; + + if (!isControlledAddress(action.action.value.swapPlaintext?.claimAddress)) { + throw new ConnectError( + "Tried to initiate a swap with a claim address belonging to a different user. This means that, when the swap is claimed, the funds would go to someone else's address, not yours. This should never happen. The website you are using may be trying to steal your funds!", + Code.PermissionDenied, + ); + } + }); +}; diff --git a/packages/router/src/grpc/custody/authorize.test.ts b/packages/router/src/grpc/custody/authorize/index.test.ts similarity index 96% rename from packages/router/src/grpc/custody/authorize.test.ts rename to packages/router/src/grpc/custody/authorize/index.test.ts index 57c653c9ce..b70c004061 100644 --- a/packages/router/src/grpc/custody/authorize.test.ts +++ b/packages/router/src/grpc/custody/authorize/index.test.ts @@ -1,9 +1,9 @@ import { beforeEach, describe, expect, Mock, test, vi } from 'vitest'; import { createContextValues, createHandlerContext, HandlerContext } from '@connectrpc/connect'; -import { approverCtx } from '../../ctx/approver'; -import { extLocalCtx, extSessionCtx, servicesCtx } from '../../ctx/prax'; -import { IndexedDbMock, MockExtLocalCtx, MockExtSessionCtx, MockServices } from '../test-utils'; -import { authorize } from './authorize'; +import { approverCtx } from '../../../ctx/approver'; +import { extLocalCtx, extSessionCtx, servicesCtx } from '../../../ctx/prax'; +import { IndexedDbMock, MockExtLocalCtx, MockExtSessionCtx, MockServices } from '../../test-utils'; +import { authorize } from '.'; import { AuthorizeRequest } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/custody/v1/custody_pb'; import { CustodyService } from '@buf/penumbra-zone_penumbra.connectrpc_es/penumbra/custody/v1/custody_connect'; import { @@ -36,7 +36,7 @@ describe('Authorize request handler', () => { mockServices = { getWalletServices: vi.fn(() => - Promise.resolve({ indexedDb: mockIndexedDb }), + Promise.resolve({ indexedDb: mockIndexedDb, viewServer: { fullViewingKey: 'fvk' } }), ) as MockServices['getWalletServices'], }; diff --git a/packages/router/src/grpc/custody/authorize.ts b/packages/router/src/grpc/custody/authorize/index.ts similarity index 72% rename from packages/router/src/grpc/custody/authorize.ts rename to packages/router/src/grpc/custody/authorize/index.ts index 856b2debea..94cd805d3b 100644 --- a/packages/router/src/grpc/custody/authorize.ts +++ b/packages/router/src/grpc/custody/authorize/index.ts @@ -1,12 +1,14 @@ -import type { Impl } from '.'; -import { extLocalCtx, extSessionCtx } from '../../ctx/prax'; -import { approverCtx } from '../../ctx/approver'; +import type { Impl } from '..'; +import { extLocalCtx, extSessionCtx, servicesCtx } from '../../../ctx/prax'; +import { approverCtx } from '../../../ctx/approver'; import { generateSpendKey } from '@penumbra-zone/wasm/src/keys'; import { authorizePlan } from '@penumbra-zone/wasm/src/build'; import { Key } from '@penumbra-zone/crypto-web/src/encryption'; import { Code, ConnectError } from '@connectrpc/connect'; import { Box } from '@penumbra-zone/types/src/box'; import { UserChoice } from '@penumbra-zone/types/src/user-choice'; +import { assertSwapClaimAddressesBelongToCurrentUser } from './assert-swap-claim-addresses-belong-to-current-user'; +import { isControlledAddress } from '@penumbra-zone/wasm/src/address'; export const authorize: Impl['authorize'] = async (req, ctx) => { if (!req.plan) throw new ConnectError('No plan included in request', Code.InvalidArgument); @@ -14,6 +16,12 @@ export const authorize: Impl['authorize'] = async (req, ctx) => { const approveReq = ctx.values.get(approverCtx); const sess = ctx.values.get(extSessionCtx); const local = ctx.values.get(extLocalCtx); + const walletServices = await ctx.values.get(servicesCtx).getWalletServices(); + + const { fullViewingKey } = walletServices.viewServer; + assertSwapClaimAddressesBelongToCurrentUser(req.plan, address => + isControlledAddress(fullViewingKey, address), + ); if (!approveReq) throw new ConnectError('Approver not found', Code.Unavailable); diff --git a/packages/router/src/grpc/test-utils.ts b/packages/router/src/grpc/test-utils.ts index cfb4ec9e94..629fa56abc 100644 --- a/packages/router/src/grpc/test-utils.ts +++ b/packages/router/src/grpc/test-utils.ts @@ -36,7 +36,7 @@ export interface ShieldedPoolMock { } export interface ViewServerMock { - fullViewingKey?: Mock; + fullViewingKey?: string; } interface MockQuerier { diff --git a/packages/router/src/grpc/view-protocol-server/index-by-address.ts b/packages/router/src/grpc/view-protocol-server/index-by-address.ts index 813c51a578..bcabbcee49 100644 --- a/packages/router/src/grpc/view-protocol-server/index-by-address.ts +++ b/packages/router/src/grpc/view-protocol-server/index-by-address.ts @@ -1,10 +1,9 @@ import type { Impl } from '.'; import { servicesCtx } from '../../ctx/prax'; -import { isControlledAddress } from '@penumbra-zone/wasm/src/address'; +import { getAddressIndexByAddress } from '@penumbra-zone/wasm/src/address'; import { Code, ConnectError } from '@connectrpc/connect'; -import { bech32Address } from '@penumbra-zone/bech32/src/address'; export const indexByAddress: Impl['indexByAddress'] = async (req, ctx) => { if (!req.address) throw new ConnectError('no address given in request', Code.InvalidArgument); @@ -13,9 +12,7 @@ export const indexByAddress: Impl['indexByAddress'] = async (req, ctx) => { viewServer: { fullViewingKey }, } = await services.getWalletServices(); - const address = bech32Address(req.address); - - const addressIndex = isControlledAddress(fullViewingKey, address); + const addressIndex = getAddressIndexByAddress(fullViewingKey, req.address); if (!addressIndex) return {}; diff --git a/packages/router/src/grpc/view-protocol-server/transaction-info-by-hash.test.ts b/packages/router/src/grpc/view-protocol-server/transaction-info-by-hash.test.ts index 5191036458..36793723c7 100644 --- a/packages/router/src/grpc/view-protocol-server/transaction-info-by-hash.test.ts +++ b/packages/router/src/grpc/view-protocol-server/transaction-info-by-hash.test.ts @@ -34,7 +34,8 @@ describe('TransactionInfoByHash request handler', () => { constants: vi.fn(), }; mockViewServer = { - fullViewingKey: vi.fn(), + fullViewingKey: + 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09', }; mockTendermint = { getTransaction: vi.fn(), @@ -59,9 +60,6 @@ describe('TransactionInfoByHash request handler', () => { url: '/mock', contextValues: createContextValues().set(servicesCtx, mockServices as unknown as Services), }); - mockViewServer.fullViewingKey?.mockReturnValueOnce( - 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09', - ); mockTransactionInfo.mockReturnValueOnce({ txp: transactionPerspective, txv: {}, diff --git a/packages/router/src/grpc/view-protocol-server/transaction-info.test.ts b/packages/router/src/grpc/view-protocol-server/transaction-info.test.ts index 06eea64d42..ce7b3e869b 100644 --- a/packages/router/src/grpc/view-protocol-server/transaction-info.test.ts +++ b/packages/router/src/grpc/view-protocol-server/transaction-info.test.ts @@ -40,7 +40,8 @@ describe('TransactionInfo request handler', () => { }; mockViewServer = { - fullViewingKey: vi.fn(), + fullViewingKey: + 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09', }; mockServices = { @@ -58,10 +59,6 @@ describe('TransactionInfo request handler', () => { contextValues: createContextValues().set(servicesCtx, mockServices as unknown as Services), }); - mockViewServer.fullViewingKey?.mockReturnValueOnce( - 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09', - ); - mockTransactionInfo.mockReturnValue({ txp: {}, txv: {}, diff --git a/packages/router/src/grpc/view-protocol-server/transaction-planner.test.ts b/packages/router/src/grpc/view-protocol-server/transaction-planner.test.ts index 9b8cf30e36..a23b70eaeb 100644 --- a/packages/router/src/grpc/view-protocol-server/transaction-planner.test.ts +++ b/packages/router/src/grpc/view-protocol-server/transaction-planner.test.ts @@ -32,7 +32,8 @@ describe('TransactionPlanner request handler', () => { constants: vi.fn(), }; mockViewServer = { - fullViewingKey: vi.fn(), + fullViewingKey: + 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09', }; mockServices = { getWalletServices: vi.fn(() => @@ -51,9 +52,6 @@ describe('TransactionPlanner request handler', () => { url: '/mock', contextValues: createContextValues().set(servicesCtx, mockServices as unknown as Services), }); - mockViewServer.fullViewingKey?.mockReturnValueOnce( - 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09', - ); req = new TransactionPlannerRequest({}); }); diff --git a/packages/wasm/src/address.test.ts b/packages/wasm/src/address.test.ts index 248cf6ff1b..ab8ad0fbcd 100644 --- a/packages/wasm/src/address.test.ts +++ b/packages/wasm/src/address.test.ts @@ -1,28 +1,54 @@ import { describe, expect, it } from 'vitest'; import { generateSpendKey, getAddressByIndex, getFullViewingKey } from './keys'; -import { isControlledAddress } from './address'; -import { bech32Address } from '@penumbra-zone/bech32/src/address'; +import { getAddressIndexByAddress, isControlledAddress } from './address'; +import { bech32ToAddress } from '@penumbra-zone/bech32/src/address'; +import { Address } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; describe('address', () => { const seedPhrase = 'benefit cherry cannon tooth exhibit law avocado spare tooth that amount pumpkin scene foil tape mobile shine apology add crouch situate sun business explain'; + const spendKey = generateSpendKey(seedPhrase); + const fullViewingKey = getFullViewingKey(spendKey); - describe('isControlledAddress()', () => { + describe('getAddressIndexByAddress()', () => { it('works with controlled addr', () => { - const spendKey = generateSpendKey(seedPhrase); - const fullViewingKey = getFullViewingKey(spendKey); const address = getAddressByIndex(fullViewingKey, 1); - expect(isControlledAddress(fullViewingKey, bech32Address(address))!.account).toBe(1); + expect(getAddressIndexByAddress(fullViewingKey, address)!.account).toBe(1); }); it('returns undefined with uncontrolled addr', () => { + const address = new Address({ + inner: bech32ToAddress( + 'penumbra1ftmn2a3hf8pxe0e48es8u9rqhny4xggq9wn2caxcjnfwfhwr5s0t3y6nzs9gx3ty5czd0sd9ssfgjt2pcxrq93yvgk2gu3ynmayuwgddkxthce8l445v8x6v07y2sjd8djcr6v', + ), + }); + + expect(getAddressIndexByAddress(fullViewingKey, address)).toBeUndefined(); + }); + }); + + describe('isControlledAddress()', () => { + it('returns true if the address is controlled', () => { + const address = getAddressByIndex(fullViewingKey, 1); + + expect(isControlledAddress(fullViewingKey, address)).toBe(true); + }); + + it('returns false if the address is not controlled', () => { const spendKey = generateSpendKey(seedPhrase); const fullViewingKey = getFullViewingKey(spendKey); - const address = - 'penumbra1ftmn2a3hf8pxe0e48es8u9rqhny4xggq9wn2caxcjnfwfhwr5s0t3y6nzs9gx3ty5czd0sd9ssfgjt2pcxrq93yvgk2gu3ynmayuwgddkxthce8l445v8x6v07y2sjd8djcr6v'; + const address = new Address({ + inner: bech32ToAddress( + 'penumbra1ftmn2a3hf8pxe0e48es8u9rqhny4xggq9wn2caxcjnfwfhwr5s0t3y6nzs9gx3ty5czd0sd9ssfgjt2pcxrq93yvgk2gu3ynmayuwgddkxthce8l445v8x6v07y2sjd8djcr6v', + ), + }); + + expect(isControlledAddress(fullViewingKey, address)).toBe(false); + }); - expect(isControlledAddress(fullViewingKey, address)).toBeUndefined(); + it('returns false if the address is undefined', () => { + expect(isControlledAddress(fullViewingKey, undefined)).toBe(false); }); }); }); diff --git a/packages/wasm/src/address.ts b/packages/wasm/src/address.ts index a74341b02f..f8713c19c0 100644 --- a/packages/wasm/src/address.ts +++ b/packages/wasm/src/address.ts @@ -1,14 +1,21 @@ import { get_short_address_by_index, is_controlled_address } from '../wasm'; -import { AddressIndex } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; +import { + Address, + AddressIndex, +} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; import { JsonValue } from '@bufbuild/protobuf'; +import { bech32Address } from '@penumbra-zone/bech32/src/address'; export const getShortAddressByIndex = (fullViewingKey: string, index: number) => get_short_address_by_index(fullViewingKey, index) as string; -export const isControlledAddress = ( +export const getAddressIndexByAddress = ( fullViewingKey: string, - address: string, + address: Address, ): AddressIndex | undefined => { - const res = is_controlled_address(fullViewingKey, address) as JsonValue; + const res = is_controlled_address(fullViewingKey, bech32Address(address)) as JsonValue; return res ? AddressIndex.fromJson(res) : undefined; }; + +export const isControlledAddress = (fullViewingKey: string, address?: Address): boolean => + address ? Boolean(is_controlled_address(fullViewingKey, bech32Address(address))) : false;