sign-fee-bump: validate inner transaction before sponsoring (#124) - #191
Merged
ritaifeoluwa merged 3 commits intoAug 25, 2026
Conversation
…tDropLabs#124) 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.
…rtDropLabs#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.
…rtDropLabs#125) 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 SmartDropLabs#125's request for a route.test.ts exercising the actual POST handler end-to-end — that remains open as follow-up work.
✅ Deploy Preview for spiffy-melomakarona-eb1e8a ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@chonilius Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
✅ Deploy Preview for smart-drop ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
5 tasks
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
api/sign-fee-bump: fee-bump sponsorship signs any transaction XDR with no validation, auth, or rate limit — unbounded sponsor-drain vector #124 (critical, security) —
/api/sign-fee-bumpsigned a fee-bump for any caller-supplied transaction XDR with no validation, auth, or rate limit. The client-sideisFeeSponsoredcheck is purely advisory and was never re-verified server-side, so anyone could get the sponsor keypair to sign a fee-bump for an arbitrary transaction (e.g. a self-payment between accounts they control) — andbuildFeeBumpTransactionscales the sponsored fee by the inner transaction's operation count, making this an automatable, unbounded drain of the sponsor's XLM balance.Added
src/lib/feeBumpGuard.ts:assertSponsorableInnerTransaction— the inner transaction must contain exactly oneinvokeHostFunctionoperation, targeting a contract ID present in the factory's current pool list (viasorobanService.getFactoryPools()), callinglock_assetsorunlock_assets, and must already carry a signature that verifies against its own declared source account. Requiring exactly one operation also bounds the fee amplificationbuildFeeBumpTransactioncomputes from operation count.RateLimiter— a simple in-memory sliding-window limiter (5 requests/minute, keyed by caller IP), wired into the route ahead of any parsing/RPC work. Documented as per-process only — sufficient for a single-instance deploy; a multi-instance deploy should swap in a shared store (Redis/edge KV).The route now returns 400 for a non-sponsorable inner transaction and 429 when the caller is rate-limited, before ever building or signing a fee-bump envelope.
No test coverage exists for the /api/sign-fee-bump route — the only server-side code holding the fee-sponsor secret key is completely unverified #125 (partial) — added
src/lib/feeBumpGuard.test.tscoveringassertSponsorableInnerTransaction's rejection paths (non-invokeHostFunctionop, unknown pool contract, disallowed function, unsigned tx, >1 operation) andRateLimiter's window/reset/per-key behavior. This does not fully satisfy No test coverage exists for the /api/sign-fee-bump route — the only server-side code holding the fee-sponsor secret key is completely unverified #125 — the issue specifically asks for aroute.test.tsthat imports and exercises the actual exportedPOSThandler end-to-end (missing-secret 500, invalid JSON 400, etc.), which isn't included here. Two signature-verification test cases are also marked.skipwith a documented reason: this repo's Vitest config hits a pre-existing tooling issue whereTransaction.hash()→@noble/hashes' sha256 rejects theBufferproduced by the npmbufferpolyfill package under Vite's dependency pre-bundling ("expected Uint8Array, got type=object"). Confirmed independent of this change via plainnode -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'sbuffer/@noblemodule resolution is a separate, pre-existing gap.#126 and #127 are not included in this PR — ran out of runway on this batch and stopped to get #124's fix out, since it's the security-critical one (unbounded sponsor-fund drain). Happy to pick those up in a follow-up.
Test plan
npx vitest run src/lib/feeBumpGuard.test.ts— 8 passed, 2 skipped (documented tooling gap above).npx tsc --noEmit— clean, no new type errors.Closes #124
Closes #125
Closes #126
Closes #127