From a1f04b27b73db2a944b8866edd933c7f72bf83bd Mon Sep 17 00:00:00 2001 From: chonilius Date: Tue, 25 Aug 2026 14:26:59 +0100 Subject: [PATCH 1/3] api/sign-fee-bump: validate inner transaction before sponsoring (#124) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route signed a fee-bump for any caller-supplied transaction XDR with no validation, auth, or rate limit — the client-side isFeeSponsored check is purely advisory and never re-verified server-side. Anyone could get the sponsor keypair to sign a fee-bump for an arbitrary transaction (e.g. a self-payment between accounts they control), and buildFeeBumpTransaction scales the sponsored fee by the inner transaction's operation count, making this an automatable, unbounded drain of the sponsor's XLM balance. Add assertSponsorableInnerTransaction: the inner transaction must contain exactly one invokeHostFunction operation, targeting a contract ID present in the factory's current pool list, calling lock_assets or unlock_assets, and must already carry a signature that verifies against its own declared source account. Requiring exactly one operation also bounds the fee amplification buildFeeBumpTransaction computes from operation count. Also add a simple in-memory sliding-window rate limiter keyed by caller IP (5 requests/minute) — noted in the module doc as per-process only, sufficient for a single-instance deploy; a multi-instance deploy should swap in a shared store. --- src/lib/feeBumpGuard.ts | 113 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 src/lib/feeBumpGuard.ts diff --git a/src/lib/feeBumpGuard.ts b/src/lib/feeBumpGuard.ts new file mode 100644 index 0000000..2b153f7 --- /dev/null +++ b/src/lib/feeBumpGuard.ts @@ -0,0 +1,113 @@ +/** + * Server-side validation for /api/sign-fee-bump (issue #124). + * + * The route sponsors real XLM spend, so before it fee-bumps and signs an + * inner transaction it must independently confirm — not just trust the + * client's advisory `isFeeSponsored` check — that the inner transaction is + * actually a SmartDrop lock/unlock call against a known pool, and that it + * already carries a valid signature from its own source account. + */ +import { Address, Keypair, StrKey, Transaction } from '@stellar/stellar-sdk'; + +export const SPONSORABLE_FUNCTIONS = new Set(['lock_assets', 'unlock_assets']); + +/** + * Throws if `innerTx` is not exactly one invokeHostFunction call to + * `lock_assets`/`unlock_assets` on one of `knownPoolContractIds`, or if it + * isn't already validly signed by its own declared source account. + */ +export function assertSponsorableInnerTransaction( + innerTx: Transaction, + knownPoolContractIds: ReadonlySet, +): void { + const operations = innerTx.operations; + if (operations.length !== 1) { + throw new Error( + `Inner transaction must contain exactly one operation, got ${operations.length}.`, + ); + } + + const op = operations[0]; + if (op.type !== 'invokeHostFunction') { + throw new Error( + `Inner transaction operation type "${op.type}" is not sponsorable; only invokeHostFunction is allowed.`, + ); + } + + const hostFn = op.func; + if (hostFn.switch().name !== 'hostFunctionTypeInvokeContract') { + throw new Error('Inner transaction must invoke a contract function.'); + } + + const invocation = hostFn.invokeContract(); + const contractId = Address.fromScAddress(invocation.contractAddress()).toString(); + const rawFunctionName = invocation.functionName(); + const functionName = + typeof rawFunctionName === 'string' ? rawFunctionName : rawFunctionName.toString('utf8'); + + if (!knownPoolContractIds.has(contractId)) { + throw new Error(`Inner transaction targets an unrecognized pool contract: ${contractId}.`); + } + + if (!SPONSORABLE_FUNCTIONS.has(functionName)) { + throw new Error( + `Inner transaction calls "${functionName}", which is not a sponsorable function.`, + ); + } + + if (innerTx.signatures.length === 0) { + throw new Error('Inner transaction is not signed.'); + } + + if (!StrKey.isValidEd25519PublicKey(innerTx.source)) { + throw new Error('Inner transaction source account is not a supported address type.'); + } + + const sourceKeypair = Keypair.fromPublicKey(innerTx.source); + const hash = innerTx.hash(); + const hasValidSourceSignature = innerTx.signatures.some((sig) => { + try { + return sourceKeypair.verify(hash, sig.signature()); + } catch { + return false; + } + }); + + if (!hasValidSourceSignature) { + throw new Error( + 'Inner transaction does not carry a valid signature from its own source account.', + ); + } +} + +/** + * Minimal in-memory sliding-window rate limiter, keyed by caller (source + * account, IP, etc). Per-process only — fine for a single-instance deploy; + * a multi-instance deploy should replace this with a shared store (e.g. + * Redis/edge KV) keyed the same way. + */ +export class RateLimiter { + private hits = new Map(); + + constructor( + private readonly maxRequests: number, + private readonly windowMs: number, + ) {} + + /** Returns true and records the hit if `key` is within its limit, false if throttled. */ + tryConsume(key: string, now: number = Date.now()): boolean { + const windowStart = now - this.windowMs; + const recent = (this.hits.get(key) ?? []).filter((t) => t > windowStart); + + if (recent.length >= this.maxRequests) { + this.hits.set(key, recent); + return false; + } + + recent.push(now); + this.hits.set(key, recent); + return true; + } +} + +export const feeBumpRateLimiter = new RateLimiter(5, 60_000); From 223473c1063d407aefd9fcd94211af2226fe563e Mon Sep 17 00:00:00 2001 From: chonilius Date: Tue, 25 Aug 2026 14:27:09 +0100 Subject: [PATCH 2/3] api/sign-fee-bump: wire in rate limiting and inner-tx validation (#124) Rejects with 429 when the caller IP exceeds the rate limit, and with 400 when assertSponsorableInnerTransaction rejects the inner transaction, before ever building or signing a fee-bump envelope. --- src/app/api/sign-fee-bump/route.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/app/api/sign-fee-bump/route.ts b/src/app/api/sign-fee-bump/route.ts index 4b3911a..05aa6dd 100644 --- a/src/app/api/sign-fee-bump/route.ts +++ b/src/app/api/sign-fee-bump/route.ts @@ -1,7 +1,8 @@ import { NextResponse } from 'next/server'; import { Keypair, TransactionBuilder, Transaction } from '@stellar/stellar-sdk'; -import { buildFeeBumpTransaction } from '@/lib/soroban'; +import { buildFeeBumpTransaction, sorobanService } from '@/lib/soroban'; import { networkPassphrase } from '@/config'; +import { assertSponsorableInnerTransaction, feeBumpRateLimiter } from '@/lib/feeBumpGuard'; export async function POST(request: Request) { try { @@ -26,6 +27,15 @@ export async function POST(request: Request) { return NextResponse.json({ error: 'Missing innerTxXdr in request body' }, { status: 400 }); } + // Rate-limit by caller IP before doing any parsing/RPC work (issue #124). + const callerIp = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || 'unknown'; + if (!feeBumpRateLimiter.tryConsume(callerIp)) { + return NextResponse.json( + { error: 'Too many fee-bump requests. Please slow down.' }, + { status: 429 }, + ); + } + // Load sponsor keypair let sponsorKeypair: Keypair; try { @@ -54,6 +64,18 @@ export async function POST(request: Request) { ); } + // Only fee-bump a signed lock_assets/unlock_assets call against a known + // pool contract — never an arbitrary caller-supplied transaction (#124). + try { + const pools = await sorobanService.getFactoryPools(); + const knownPoolContractIds = new Set(pools.map((pool) => pool.contractAddress)); + assertSponsorableInnerTransaction(innerTxObj, knownPoolContractIds); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + console.warn('[SignFeeBump] Rejected non-sponsorable inner transaction:', msg); + return NextResponse.json({ error: msg }, { status: 400 }); + } + // Build the fee-bump transaction const feeBumpTx = buildFeeBumpTransaction( innerTxObj, From e6778dae18bba64db4a8f574f4ab202445c8382c Mon Sep 17 00:00:00 2001 From: chonilius Date: Tue, 25 Aug 2026 14:27:20 +0100 Subject: [PATCH 3/3] feeBumpGuard: add test coverage for validation and rate limiting (#125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers assertSponsorableInnerTransaction's rejection paths (non-invokeHost operation, unknown pool contract, disallowed function, unsigned tx, >1 operation) and RateLimiter's window/reset/per-key behavior. Two signature-verification cases are marked .skip with a documented reason: this repo's Vitest config hits a pre-existing tooling issue where Transaction.hash() -> @noble/hashes sha256 rejects the Buffer produced by the npm `buffer` polyfill package under Vite's dependency pre-bundling ("expected Uint8Array, got type=object"). Confirmed independent of this change via plain `node -e`, where the identical stellar-sdk calls (signing, verification, tx.hash()) all succeed — and Next.js API routes run under Node directly, not Vite, so production is unaffected. Fixing Vitest's buffer/@noble module resolution is a separate pre-existing gap. This does not fully satisfy #125's request for a route.test.ts exercising the actual POST handler end-to-end — that remains open as follow-up work. --- src/lib/feeBumpGuard.test.ts | 174 +++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 src/lib/feeBumpGuard.test.ts diff --git a/src/lib/feeBumpGuard.test.ts b/src/lib/feeBumpGuard.test.ts new file mode 100644 index 0000000..b7152bb --- /dev/null +++ b/src/lib/feeBumpGuard.test.ts @@ -0,0 +1,174 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + Account, + Address, + Asset, + Contract, + Keypair, + Networks, + Operation, + StrKey, + TransactionBuilder, + nativeToScVal, + xdr, +} from '@stellar/stellar-sdk'; +import { assertSponsorableInnerTransaction, RateLimiter } from './feeBumpGuard'; + +// NOTE: the two cases that reach signature verification (`.skip`d below) +// hit a pre-existing Vitest-only issue in this repo: `Transaction.hash()` +// calls into @noble/hashes' sha256 with a Buffer produced via the npm +// `buffer` polyfill package, and under Vite/Vitest's dependency +// pre-bundling that Buffer fails @noble/hashes' `abytes` Uint8Array check +// ("expected Uint8Array, got type=object") — confirmed independent of this +// change: the identical stellar-sdk calls (Keypair construction, signing, +// verification, tx.hash()) all succeed under plain `node -e` outside +// Vite/Vitest, and Next.js API routes run under Node directly (not Vite), +// so this does not affect the real route. Fixing Vitest's module +// resolution for `buffer`/`@noble/*` is a separate, pre-existing +// tooling gap outside the scope of this change. +function attachDummySignature(tx: { addDecoratedSignature: (sig: xdr.DecoratedSignature) => void }) { + tx.addDecoratedSignature( + new xdr.DecoratedSignature({ + hint: Buffer.alloc(4, 9), + signature: Buffer.alloc(64, 9), + }), + ); +} + +const POOL_ID = StrKey.encodeContract(new Uint8Array(32).fill(1)); +const OTHER_CONTRACT_ID = StrKey.encodeContract(new Uint8Array(32).fill(2)); +const SOURCE_PUBKEY = StrKey.encodeEd25519PublicKey(new Uint8Array(32).fill(3)); + +function buildLockAssetsTx(poolId: string) { + const account = new Account(SOURCE_PUBKEY, '0'); + const contract = new Contract(poolId); + const operation = contract.call( + 'lock_assets', + Address.fromString(SOURCE_PUBKEY).toScVal(), + nativeToScVal(10_000_000, { type: 'i128' }), + ); + return new TransactionBuilder(account, { fee: '100', networkPassphrase: Networks.TESTNET }) + .addOperation(operation) + .setTimeout(300) + .build(); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('assertSponsorableInnerTransaction', () => { + it.skip('accepts a signed lock_assets call against a known pool (blocked by Vitest buffer/noble tooling gap, see note above)', () => { + vi.spyOn(Keypair.prototype, 'verify').mockReturnValue(true); + const tx = buildLockAssetsTx(POOL_ID); + attachDummySignature(tx); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).not.toThrow(); + }); + + it('rejects a plain Payment operation', () => { + const account = new Account(SOURCE_PUBKEY, '0'); + const tx = new TransactionBuilder(account, { fee: '100', networkPassphrase: Networks.TESTNET }) + .addOperation( + Operation.payment({ + destination: StrKey.encodeEd25519PublicKey(new Uint8Array(32).fill(4)), + asset: Asset.native(), + amount: '1', + }), + ) + .setTimeout(300) + .build(); + attachDummySignature(tx); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).toThrow( + /not sponsorable/, + ); + }); + + it('rejects an invokeHostFunction call targeting a contract not in the known pool set', () => { + const tx = buildLockAssetsTx(OTHER_CONTRACT_ID); + attachDummySignature(tx); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).toThrow( + /unrecognized pool contract/, + ); + }); + + it('rejects a call to a function outside the sponsorable allow-list', () => { + const account = new Account(SOURCE_PUBKEY, '0'); + const contract = new Contract(POOL_ID); + const operation = contract.call('drain_pool', Address.fromString(SOURCE_PUBKEY).toScVal()); + const tx = new TransactionBuilder(account, { fee: '100', networkPassphrase: Networks.TESTNET }) + .addOperation(operation) + .setTimeout(300) + .build(); + attachDummySignature(tx); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).toThrow( + /not a sponsorable function/, + ); + }); + + it('rejects an unsigned inner transaction', () => { + const tx = buildLockAssetsTx(POOL_ID); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).toThrow( + /not signed/, + ); + }); + + it.skip('rejects a transaction whose attached signature does not verify against its source (blocked by Vitest buffer/noble tooling gap, see note above)', () => { + vi.spyOn(Keypair.prototype, 'verify').mockReturnValue(false); + const tx = buildLockAssetsTx(POOL_ID); + attachDummySignature(tx); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).toThrow( + /valid signature/, + ); + }); + + it('rejects a transaction with more than one operation', () => { + const account = new Account(SOURCE_PUBKEY, '0'); + const contract = new Contract(POOL_ID); + const operation = contract.call( + 'lock_assets', + Address.fromString(SOURCE_PUBKEY).toScVal(), + nativeToScVal(10_000_000, { type: 'i128' }), + ); + const tx = new TransactionBuilder(account, { fee: '100', networkPassphrase: Networks.TESTNET }) + .addOperation(operation) + .addOperation(operation) + .setTimeout(300) + .build(); + attachDummySignature(tx); + + expect(() => assertSponsorableInnerTransaction(tx, new Set([POOL_ID]))).toThrow( + /exactly one operation/, + ); + }); +}); + +describe('RateLimiter', () => { + it('allows up to the configured max requests within the window, then throttles', () => { + const limiter = new RateLimiter(2, 1000); + const now = 1_000_000; + expect(limiter.tryConsume('a', now)).toBe(true); + expect(limiter.tryConsume('a', now + 10)).toBe(true); + expect(limiter.tryConsume('a', now + 20)).toBe(false); + }); + + it('resets once the window has elapsed', () => { + const limiter = new RateLimiter(1, 1000); + const now = 1_000_000; + expect(limiter.tryConsume('a', now)).toBe(true); + expect(limiter.tryConsume('a', now + 500)).toBe(false); + expect(limiter.tryConsume('a', now + 1001)).toBe(true); + }); + + it('tracks distinct keys independently', () => { + const limiter = new RateLimiter(1, 1000); + const now = 1_000_000; + expect(limiter.tryConsume('a', now)).toBe(true); + expect(limiter.tryConsume('b', now)).toBe(true); + }); +});