Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 189 additions & 0 deletions src/lib/soroban.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
unlockAssets,
validateSimulationAuth,
} from "./soroban";
import { SecurityError } from "./error-handler";

const POOL_CONTRACT_ID =
"CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABSC4";
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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 });
Expand Down
30 changes: 29 additions & 1 deletion src/lib/soroban.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ type FreighterSignTransactionResult =
export interface FreighterWalletApi {
signTransaction: (
transactionXdr: string,
options: { networkPassphrase: string },
options: { networkPassphrase: string; address?: string },
) => Promise<FreighterSignTransactionResult>;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -939,7 +961,9 @@ export class SorobanService {
const signedTransaction = getSignedTransactionXdr(
await walletApi.signTransaction(preparedTransaction.toXDR(), {
networkPassphrase,
address: userAddress,
}),
userAddress,
);

let finalTxXdr = signedTransaction;
Expand Down Expand Up @@ -1080,7 +1104,9 @@ export class SorobanService {
const signedTransaction = getSignedTransactionXdr(
await walletApi.signTransaction(preparedTransaction.toXDR(), {
networkPassphrase,
address: userAddress,
}),
userAddress,
);

let finalTxXdr = signedTransaction;
Expand Down Expand Up @@ -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(
Expand Down
Loading