fix(airdrop): canonicalize Base wallet case so one wallet cannot double-claim#8010
Conversation
…le-claim Base (EVM) addresses are case-insensitive on-chain — 0xAbCd..., 0xabcd... and 0xABCD... are the same account and eth_getBalance returns the same balance for every casing. But the one-airdrop-per-wallet invariant (_has_claimed + UNIQUE(github_username, wallet_address, chain)) compared the wallet as a raw string, so the same physical Base wallet could collect the airdrop once per GitHub identity just by varying hex case, draining the Base allocation. Add _normalize_wallet() and canonicalize wallet_address (lowercase for base; base58/Solana left untouched, it is case-sensitive) at the entry of both check_eligibility and claim_airdrop, so dedup, the UNIQUE index, the generated claim id and the RPC balance check all use one canonical form. Regression test fails on main (same Base wallet double-claims -> 100 wRTC charged) and passes with the fix (50 wRTC, one claim); Solana distinct-case wallets stay independently claimable.
|
/claim rustchain-bounties#71 — verified bug: same Base (EVM) wallet double-claims the airdrop by varying hex case (wallet-dedup invariant void for the whole Base chain). Deterministic regression test included; existing suite 31/31 green. Payout RTC: RTCd1554f0f35576faf01d386a6be1c947f560dd0b7 |
jjb9707
left a comment
There was a problem hiding this comment.
Substantive review (Bounty #73 candidate)
Verified scope: node/airdrop_v2.py adds chain-aware wallet canonicalization and node/test_airdrop_wallet_case_dedup.py adds three regression paths. I compared the patch with the actual schema, _has_claimed, eligibility, claim-record, and allocation-update paths at head eb2641e64624fbbdf4eb07053c6f4410997822d5.
Strengths
- The threat model is correct and narrowly scoped: Base/EVM addresses are case-insensitive, while Solana base58 addresses are case-sensitive. Lowercasing only
chain == "base"avoids corrupting Solana identities. - Canonicalization occurs before both
_has_claimed()call sites and before theClaimRecord/claim ID are built. Consequently the lookup, stored wallet, RPC validation input, generated ID, and future comparisons all share one representation rather than merely normalizing one query. - The regression suite covers the two externally visible paths (
check_eligibilityandclaim_airdrop) and explicitly protects the Solana non-regression. The allocation assertion also proves rejected case variants do not debit the pool.
Substantive findings
- The existing SQL uniqueness constraint does not independently enforce one-wallet/one-account semantics.
UNIQUE(github_username, wallet_address, chain)only rejects the same pair;_has_claimed()is what enforces the OR rule. This patch correctly closes the reported sequential case-variant exploit, but two concurrent requests for different GitHub users targeting the same canonical Base wallet can still both pass_has_claimed()before either insert, and both distinct tuples can commit. That is a pre-existing race, not introduced here. A follow-up migration should add separate unique indexes for active claims (or serialize the check/insert with an immediate transaction) so the database owns the invariant. - Legacy mixed-case rows are not migrated. New writes are canonical, and new checks catch an exact stored casing, but a database that already contains
0xAbCd...will not match a new canonical lowercase query. Before rollout, normalize existing Base rows and resolve any collisions, or temporarily comparelower(wallet_address)for Base until the migration is complete. This matters because the patch changes application semantics without rewriting historical state. - Validation ordering is acceptable but worth preserving deliberately.
_normalize_wallet()runs after the supported-chain guard incheck_eligibility, whileclaim_airdrop()normalizes immediately afterchain.lower(). Since unsupported chains are not lowercased by the helper, there is no cross-chain corruption; however, any future direct storage path must call the same helper or the invariant can regress. Centralizing canonicalization at a typed wallet boundary would make that harder.
Verification
- All 12 reported checks on the reviewed head completed successfully, including
test,BCOS Tier Gate,BCOS v2 Engine Scan, and the review-tier gate. - Read-through confirms the canonical value is passed into
_check_wallet, stored inairdrop_claims, included inclaim_id, and used by_has_claimed. - The new tests are deterministic (
:memory:+skip_antisybil=True) and assert both rejection behavior and allocation accounting.
Verdict: APPROVE — the patch fixes the demonstrated sequential Base-address casing bypass without changing Solana semantics. The legacy-row migration and database-level concurrency invariant should be tracked as follow-ups; neither invalidates the focused fix for fresh canonical writes.
Reviewer: @jjb9707
Bug: same Base (EVM) wallet can claim the airdrop multiple times by varying hex case
node/airdrop_v2.pyenforces one airdrop per GitHub account and per wallet —_has_claimed()andUNIQUE(github_username, wallet_address, chain). Only the GitHub half is canonicalized (_normalize_github_usernamecasefolds it);wallet_addressis compared as a raw string everywhere.Base is EVM, where addresses are case-insensitive on-chain:
0xAbCd…,0xabcd…and0xABCD…are the same account andeth_getBalance(_check_base_wallet) returns the identical balance for every casing — so every casing passes the anti-Sybil check. But_has_claimedand theUNIQUEindex treat each casing as a different wallet, so the wallet-dedup leg is void for the entire Base chain.Impact
N aged, PR-merged GitHub accounts can all funnel the airdrop to one physical Base wallet by only changing hex case, and the global Base allocation is debited each time. Even a single honest user who claims with an EIP-55 checksummed address and re-submits the lowercased form gets a second allocation.
Repro (deterministic, no network —
skip_antisybil=True)Fix
Add
_normalize_wallet(address, chain)and canonicalizewallet_addressat the entry of bothcheck_eligibilityandclaim_airdrop(lowercase forbase; Solana base58 left untouched — it is genuinely case-sensitive), so dedup, the UNIQUE index, the generated claim id and the RPC balance check all use one canonical form.Tests
node/test_airdrop_wallet_case_dedup.py: fails on main (Base wallet double-claims, 100 wRTC), passes with fix (50 wRTC, one claim); Solana distinct-case wallets stay independently claimable.node/test_airdrop_v2.py— 31 passed, no regressions.Claiming under rustchain-bounties#71. Payout RTC:
RTCd1554f0f35576faf01d386a6be1c947f560dd0b7