fix: verify wallet ownership in bridge estimate and rebalance - #26
Open
memosr wants to merge 1 commit into
Open
Conversation
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
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.
Problem
app/api/bridge/rebalance/route.tsandapp/api/bridge/estimate/route.tsauthenticate the caller but acceptsourceWalletIdanddestinationWalletIdfrom the POST body without verifying those wallets belong to the authenticated user.Why this matters
The Circle Developer SDK operates with an app-level API key that has signing authority over every wallet in the developer account — there is no per-user scope at the SDK level. The ownership boundary is enforced only by the
walletstable in Supabase, which these two handlers never query for authorization.Exploit
User A (authenticated) sends
POST /api/bridge/rebalancewith:{ "sourceWalletId": "<User B's circle_wallet_id>", "destinationWalletId": "<User A's circle_wallet_id>", ... }The handler validates User A's session, fetches both wallets from Circle (succeeds — both are in the same developer account), and executes the bridge transfer. User B's USDC is burned on the source chain and minted into User A's wallet. The on-chain burn is irreversible once the CCTP attestation is submitted. The inserted transaction record carries
user_id: user.id(User A), so it never appears in User B's history./api/bridge/estimatehas the same missing ownership check, allowing User A to silently probe User B's wallet balances before the attack.By contrast,
app/api/wallet/transfer/route.ts:52-57andapp/api/gateway/deposit/route.ts:76-82correctly scope their Supabase query with.eq("user_id", user.id)before touching Circle.Fix
Added ownership checks for both
sourceWalletIdanddestinationWalletIdbefore any Circle SDK call. Pattern matchestransfer/route.ts:Same change applied to both
bridge/estimate/route.tsandbridge/rebalance/route.ts.Impact
estimate(no more probing other users' balances).wallet/transfer/route.ts.