Skip to content

fix(wallet): H2 empty-asset cap bypass; M16 capMu held across settler round-trip - #29

Merged
TeoSlayer merged 1 commit into
mainfrom
sec/findings-wallet
Jul 26, 2026
Merged

fix(wallet): H2 empty-asset cap bypass; M16 capMu held across settler round-trip#29
TeoSlayer merged 1 commit into
mainfrom
sec/findings-wallet

Conversation

@TeoSlayer

Copy link
Copy Markdown
Contributor

Addresses findings H2 and M16 from the audit register.

H2 — empty-asset x402 contract bypassed the USDC spend cap

pkg/wallet/hooks_evm.go

An x402 payment.Contract that leaves Asset empty is denominated in the chain's USDC — that is the only token EVMMethod.Satisfy will ever authorize (it accepts "" and signs against the USDC contract). The cap check compared the raw field, so a SpendCap{Asset: "USDC"} never matched and the spend was neither gated nor recorded.

NormalizeEVMAsset resolves the field once, and is applied both at the IPC boundary (walletipc.evmSatisfyHandler) and inside SatisfyEVMOn, so the cap check, the recorded spend and the signer all see the same symbol regardless of caller.

M16 — capMu held across the settler network round-trip

pkg/wallet/hooks_settler.go

SettlerTransfer held capMu for the whole settler call, so a slow or unresponsive settler stalled every other cap-gated path on the wallet (Pay, SatisfyEVM, even SpendCaps()).

Simply dropping the lock around the call would reintroduce a TOCTOU — two concurrent transfers could each pass a check against the same budget. Instead the check now claims its budget as an in-flight reservation under the same lock acquisition; the lock is released for the round-trip; then the reservation is either promoted to a recorded spend (success) or returned (failure). Reservations count toward spentInWindowLocked, so check-and-claim stays atomic.

Tests

pkg/wallet/zz_cap_boundary_test.go — all three fail on main, pass here:

  • over-cap empty-asset satisfy is rejected (was silently allowed)
  • within-cap empty-asset satisfy books against USDC (was booked nowhere)
  • SpentInWindow stays responsive while a transfer is outstanding, the in-flight amount gates a concurrent transfer, and a failed transfer returns its reservation

No wire-format or protocol changes.

🤖 Generated with Claude Code

…pMu during settler calls

An x402 contract that leaves Asset empty is denominated in the chain's
USDC — the only token the EVM signer will authorize. The cap check
compared the raw field, so a USDC-scoped cap never matched an
empty-asset contract and the spend went through uncounted. Asset now
resolves through NormalizeEVMAsset at the IPC boundary and again in
SatisfyEVMOn, so cap accounting and the signer see the same symbol.

SettlerTransfer held capMu across the settler round-trip, so a slow or
unresponsive settler stalled every other cap-gated path on the wallet.
The check now claims its budget as an in-flight reservation under the
same lock acquisition, releases the lock for the round-trip, then either
promotes the reservation to a recorded spend or returns it. Reservations
count toward spentInWindow, so check-and-claim stays atomic and a
concurrent caller cannot be granted budget an outstanding transfer
already holds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/wallet/spendcap.go 73.33% 2 Missing and 2 partials ⚠️
pkg/wallet/hooks_settler.go 75.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@TeoSlayer
TeoSlayer merged commit e0e1bec into main Jul 26, 2026
3 checks passed
@TeoSlayer
TeoSlayer deleted the sec/findings-wallet branch July 26, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants