diff --git a/src/lib/soroban.service.test.ts b/src/lib/soroban.service.test.ts index fd9208c..97f8407 100644 --- a/src/lib/soroban.service.test.ts +++ b/src/lib/soroban.service.test.ts @@ -49,6 +49,7 @@ import { unlockAssets, validateSimulationAuth, } from "./soroban"; +import { SecurityError } from "./error-handler"; const POOL_CONTRACT_ID = "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABSC4"; @@ -732,6 +733,7 @@ describe("SorobanService RPC writes", () => { ); expect(walletApi.signTransaction).toHaveBeenCalledWith(expect.any(String), { networkPassphrase: expect.any(String), + address: USER_PUBLIC_KEY, }); expect(rpcServer.sendTransaction).toHaveBeenCalledTimes(1); }); @@ -955,6 +957,193 @@ describe("SorobanService RPC writes", () => { }); }); +describe("SorobanService signer address pinning (#139)", () => { + const OTHER_PUBLIC_KEY = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 3)); + + it("lockAssets rejects a transaction signed by a different Freighter account", async () => { + vi.spyOn(console, "error").mockImplementation(() => undefined); + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("lock_assets")] }, + minResourceFee: "321", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => ({ + signedTxXdr: xdrEnvelope, + signerAddress: OTHER_PUBLIC_KEY, + })), + }; + + await expect( + service.lockAssets(POOL_ID, USER_PUBLIC_KEY, "50000000", walletApi), + ).rejects.toThrow(SecurityError); + expect(walletApi.signTransaction).toHaveBeenCalledWith(expect.any(String), { + networkPassphrase: expect.any(String), + address: USER_PUBLIC_KEY, + }); + expect(rpcServer.sendTransaction).not.toHaveBeenCalled(); + }); + + it("lockAssets proceeds when the signer address matches the connected account", async () => { + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("lock_assets")] }, + minResourceFee: "321", + }); + rpcServer.sendTransaction.mockResolvedValue({ + status: "PENDING", + hash: "lock-hash", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => ({ + signedTxXdr: xdrEnvelope, + signerAddress: USER_PUBLIC_KEY, + })), + }; + + const result = await service.lockAssets( + POOL_ID, + USER_PUBLIC_KEY, + "50000000", + walletApi, + ); + + expect(result).toMatchObject({ success: true, transactionHash: "lock-hash" }); + expect(rpcServer.sendTransaction).toHaveBeenCalledTimes(1); + }); + + it("unlockAssets rejects a transaction signed by a different Freighter account", async () => { + vi.spyOn(console, "error").mockImplementation(() => undefined); + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("unlock_assets")] }, + minResourceFee: "654", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => ({ + signedTxXdr: xdrEnvelope, + signerAddress: OTHER_PUBLIC_KEY, + })), + }; + + await expect( + service.unlockAssets(POOL_ID, USER_PUBLIC_KEY, "25000000", walletApi), + ).rejects.toThrow(SecurityError); + expect(walletApi.signTransaction).toHaveBeenCalledWith(expect.any(String), { + networkPassphrase: expect.any(String), + address: USER_PUBLIC_KEY, + }); + expect(rpcServer.sendTransaction).not.toHaveBeenCalled(); + }); + + it("unlockAssets proceeds when the signer address matches the connected account", async () => { + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("unlock_assets")] }, + minResourceFee: "654", + }); + rpcServer.sendTransaction.mockResolvedValue({ + status: "PENDING", + hash: "unlock-hash", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => ({ + signedTxXdr: xdrEnvelope, + signerAddress: USER_PUBLIC_KEY, + })), + }; + + const result = await service.unlockAssets( + POOL_ID, + USER_PUBLIC_KEY, + "25000000", + walletApi, + ); + + expect(result).toMatchObject({ success: true, transactionHash: "unlock-hash" }); + expect(rpcServer.sendTransaction).toHaveBeenCalledTimes(1); + }); + + it("setBoost rejects a transaction signed by a different Freighter account", async () => { + vi.spyOn(console, "error").mockImplementation(() => undefined); + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("set_boost")] }, + minResourceFee: "777", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => ({ + signedTxXdr: xdrEnvelope, + signerAddress: OTHER_PUBLIC_KEY, + })), + }; + + await expect( + service.setBoost(POOL_ID, USER_PUBLIC_KEY, 40, walletApi), + ).rejects.toThrow(SecurityError); + expect(walletApi.signTransaction).toHaveBeenCalledWith(expect.any(String), { + networkPassphrase: expect.any(String), + address: USER_PUBLIC_KEY, + }); + expect(rpcServer.sendTransaction).not.toHaveBeenCalled(); + }); + + it("setBoost proceeds when the signer address matches the connected account", async () => { + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("set_boost")] }, + minResourceFee: "777", + }); + rpcServer.sendTransaction.mockResolvedValue({ + status: "PENDING", + hash: "boost-hash", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => ({ + signedTxXdr: xdrEnvelope, + signerAddress: USER_PUBLIC_KEY, + })), + }; + + const result = await service.setBoost(POOL_ID, USER_PUBLIC_KEY, 40, walletApi); + + expect(result).toMatchObject({ success: true, transactionHash: "boost-hash" }); + expect(rpcServer.sendTransaction).toHaveBeenCalledTimes(1); + }); + + it("does not block legacy bare-string signTransaction responses that omit signerAddress", async () => { + const { service, rpcServer } = makeService(); + mockAssembleTransactionPassthrough(); + rpcServer.simulateTransaction.mockResolvedValue({ + result: { auth: [makeAuthEntry("lock_assets")] }, + minResourceFee: "321", + }); + rpcServer.sendTransaction.mockResolvedValue({ + status: "PENDING", + hash: "lock-hash", + }); + const walletApi = { + signTransaction: vi.fn(async (xdrEnvelope: string) => xdrEnvelope), + }; + + const result = await service.lockAssets( + POOL_ID, + USER_PUBLIC_KEY, + "50000000", + walletApi, + ); + + expect(result).toMatchObject({ success: true, transactionHash: "lock-hash" }); + expect(rpcServer.sendTransaction).toHaveBeenCalledTimes(1); + }); +}); + describe("SorobanService platform stats", () => { it("getPlatformStats aggregates pool totals from getFactoryPools", async () => { const { service } = makeService({ pool: false }); diff --git a/src/lib/soroban.ts b/src/lib/soroban.ts index 0810187..2bd4853 100644 --- a/src/lib/soroban.ts +++ b/src/lib/soroban.ts @@ -149,7 +149,7 @@ type FreighterSignTransactionResult = export interface FreighterWalletApi { signTransaction: ( transactionXdr: string, - options: { networkPassphrase: string }, + options: { networkPassphrase: string; address?: string }, ) => Promise; } @@ -297,8 +297,24 @@ export async function simulateLockAssets( }; } +/** + * Unwraps Freighter's signTransaction response, verifying that the account + * which actually signed (`result.signerAddress`) is the account SmartDrop + * believes is connected (`expectedSigner`). Passing `address` in the + * request (see the three call sites below) lets Freighter itself refuse a + * mismatched-account attempt before ever producing a signature, but that's + * a request the extension can choose to honor or not — this is the + * client-side backstop that fails fast with a clear error instead of + * letting a signer mismatch surface only as an opaque on-chain + * authorization failure after a real transaction submission (#139). + * + * Older Freighter responses (or the legacy bare-string return shape) may + * not include `signerAddress` at all — nothing to check against in that + * case, so this only rejects when the field is present and disagrees. + */ function getSignedTransactionXdr( result: FreighterSignTransactionResult, + expectedSigner: string, ): string { if (typeof result === 'string') { return result; @@ -312,6 +328,12 @@ function getSignedTransactionXdr( ); } + if (result.signerAddress && result.signerAddress !== expectedSigner) { + throw new SecurityError( + `Transaction signing was blocked because it was signed by ${result.signerAddress}, not the connected account ${expectedSigner}. Please sign with the correct Freighter account.`, + ); + } + if (result.signedTxXdr) { return result.signedTxXdr; } @@ -939,7 +961,9 @@ export class SorobanService { const signedTransaction = getSignedTransactionXdr( await walletApi.signTransaction(preparedTransaction.toXDR(), { networkPassphrase, + address: userAddress, }), + userAddress, ); let finalTxXdr = signedTransaction; @@ -1080,7 +1104,9 @@ export class SorobanService { const signedTransaction = getSignedTransactionXdr( await walletApi.signTransaction(preparedTransaction.toXDR(), { networkPassphrase, + address: userAddress, }), + userAddress, ); let finalTxXdr = signedTransaction; @@ -1209,7 +1235,9 @@ export class SorobanService { const signedTransaction = getSignedTransactionXdr( await walletApi.signTransaction(preparedTransaction.toXDR(), { networkPassphrase, + address: userAddress, }), + userAddress, ); const submissionResult = await this.rpcServer.sendTransaction(