Skip to content

lockAssets/unlockAssets/setBoost never pin address in Freighter's signTransaction call and never validate the returned signerAddress #139

Description

@prodbycorne

Overview

@stellar/freighter-api (this project's dependency, ^3.1.0) exposes signTransaction with an optional address field that pins which of the extension's accounts must sign, and its response includes a signerAddress field confirming which account actually did:

// @stellar/freighter-api's own type surface (signTransaction.d.ts)
export declare const signTransaction: (transactionXdr: string, opts?: {
    networkPassphrase?: string;
    address?: string;
}) => Promise<{
    signedTxXdr: string;
    signerAddress: string;
} & { error?: FreighterApiError }>;

This codebase's own FreighterWalletApi interface never models or uses either field:

// src/lib/soroban.ts:149-154
export interface FreighterWalletApi {
  signTransaction: (
    transactionXdr: string,
    options: { networkPassphrase: string },
  ) => Promise<FreighterSignTransactionResult>;
}

Every real call site — lockAssets (soroban.ts:939-943), unlockAssets (soroban.ts:1080-1084), setBoost (soroban.ts:1209-1213) — passes only { networkPassphrase }, never address: userAddress, so Freighter is free to sign with whichever account is currently "active" inside the extension, not necessarily the userAddress/publicKey this app believes is connected. Worse, getSignedTransactionXdr (soroban.ts:300-320), the function that unwraps Freighter's response, only ever reads result.signedTxXdr — it never reads or checks result.signerAddress against the userAddress the caller intended:

function getSignedTransactionXdr(result: FreighterSignTransactionResult): string {
  if (typeof result === 'string') return result;
  if (result.error) { throw new Error(...); }
  if (result.signedTxXdr) { return result.signedTxXdr; }   // signerAddress silently discarded
  throw new Error('Freighter did not return a signed transaction XDR');
}

Since the built transaction embeds userAddress as the required authorizing party (e.g. Address.fromString(args.publicKey).toScVal() in buildLockAssetsTransaction), a mismatched signer would generally cause the on-chain authorization check to fail rather than silently succeed for the wrong account — but SmartDrop itself has zero client-side defense-in-depth here: it neither pins the signer up front (which would let Freighter itself reject a mismatched-account attempt before ever producing a signature) nor verifies the signer after the fact (which would let SmartDrop surface a fast, clear, client-side error — "you signed with a different account than the one connected" — instead of spending a real transaction submission on an authorization failure discovered only on-chain).

Requirements

  • Pass address: userAddress in the options object of every walletApi.signTransaction(...) call (lockAssets, unlockAssets, setBoost), so Freighter itself enforces the correct signer up front where its own API supports doing so.
  • Update FreighterWalletApi's interface and getSignedTransactionXdr to read and validate result.signerAddress === userAddress, throwing a clear, actionable SecurityError/FreighterError if they diverge, before ever calling sendTransaction.
  • Surface this as a distinct, user-legible error message ("You signed with a different Freighter account than the one connected to SmartDrop") rather than letting a mismatch surface only as an opaque on-chain authorization failure.

Acceptance Criteria

  • lockAssets/unlockAssets/setBoost all pass address: userAddress to walletApi.signTransaction.
  • getSignedTransactionXdr (or its caller) rejects with a clear error if result.signerAddress is present and doesn't match the expected userAddress, before any sendTransaction call is made.
  • A test with a mocked walletApi.signTransaction returning a signerAddress different from the userAddress passed into lockAssets asserts the flow fails fast with the new, specific error, and that sendTransaction is never called.
  • Existing happy-path behavior (signer matches expected address) is unaffected.

Additional Notes

More precise references

  • src/lib/soroban.ts:149-154 (FreighterWalletApi interface) — confirmed no address option or signerAddress response field is modeled.
  • src/lib/soroban.ts:141-147 (FreighterSignTransactionResult type) — confirmed the union type (string | { signedTxXdr?, signerAddress?, error? }) already has an optional signerAddress?: unknown slot in its shape (implicitly, via being a loosely-typed object) that is simply never read.
  • src/lib/soroban.ts:300-320 (getSignedTransactionXdr) — confirmed line-by-line that only result.signedTxXdr is ever accessed; signerAddress is never referenced anywhere in this function or its callers.
  • src/lib/soroban.ts:939-943 (lockAssets), :1080-1084 (unlockAssets), :1209-1213 (setBoost) — confirmed all three call walletApi.signTransaction(preparedTransaction.toXDR(), { networkPassphrase }) with no address field, despite userAddress/publicKey being in scope at each call site.
  • Confirmed via the installed @stellar/freighter-api type declarations (signTransaction.d.ts, matching the ^3.1.0 version pinned in package.json) that both address (request) and signerAddress (response) are real, current parts of the library's public API surface, not a hypothetical/future capability.

Additional edge cases

Implementation sketch

export interface FreighterWalletApi {
  signTransaction: (
    transactionXdr: string,
    options: { networkPassphrase: string; address?: string },
  ) => Promise<FreighterSignTransactionResult>;
}

function getSignedTransactionXdr(result: FreighterSignTransactionResult, expectedSigner: string): string {
  if (typeof result === 'string') return result; // no signerAddress to check in the legacy string-return shape
  if (result.error) { throw new Error(...); }
  if (result.signerAddress && result.signerAddress !== expectedSigner) {
    throw new SecurityError(
      `Transaction was signed by ${result.signerAddress}, not the connected account ${expectedSigner}. Please sign with the correct Freighter account.`,
    );
  }
  if (result.signedTxXdr) return result.signedTxXdr;
  throw new Error('Freighter did not return a signed transaction XDR');
}

// call sites:
await walletApi.signTransaction(preparedTransaction.toXDR(), { networkPassphrase, address: userAddress });

Test/reproduction plan

  • Mock walletApi.signTransaction to resolve { signedTxXdr: '...', signerAddress: 'GDIFFERENT...' } while lockAssets is called with userAddress = 'GEXPECTED...'; assert the call throws the new mismatch error and rpcServer.sendTransaction/this.rpcServer.sendTransaction is never invoked.
  • Mock a matching signerAddress; assert the flow proceeds normally through to submission.
  • Assert walletApi.signTransaction was called with address: userAddress in its options object, for each of lockAssets/unlockAssets/setBoost.

Cross-references

Metadata

Metadata

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial CampaignCampaign: Official CampaignOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaignsecuritySecurity, signing safety, or wallet interaction hardeningvery hardExtremely hard — deep expertise, careful design, and significant time requiredwalletFreighter wallet integration, session, and network switching

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions