Skip to content

fix(airdrop): same Base wallet can claim twice via EIP-55 checksum casing (anti-Sybil bypass)#7974

Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Scottcjn:mainfrom
Vyacheslav-Tomashevskiy:fix/airdrop-evm-wallet-case-double-claim
Open

fix(airdrop): same Base wallet can claim twice via EIP-55 checksum casing (anti-Sybil bypass)#7974
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Scottcjn:mainfrom
Vyacheslav-Tomashevskiy:fix/airdrop-evm-wallet-case-double-claim

Conversation

@Vyacheslav-Tomashevskiy

Copy link
Copy Markdown
Contributor

Problem

_has_claimed normalizes the GitHub username but compares the wallet byte-exactly:

github_username = self._normalize_github_username(github_username)   # .strip().casefold()
...
AND (github_username = ? OR wallet_address = ?)

EVM addresses are case-insensitive — the mixed-case form is only an EIP-55 display checksum over the same 20 bytes. So 0x5683C105…9c6 and 0x5683c105…9c6 are the same Base wallet, but they read as two different ones. UNIQUE(github_username, wallet_address, chain) doesn't catch it either, since the stored strings genuinely differ.

Net effect: RIP-305's "One claim per wallet address" is bypassable on Base. N GitHub accounts can funnel N × 200 wRTC into a single wallet by varying only the casing. Reproduced: claimed_uwrtc for base reaches 400 wRTC for one wallet.

Why this reads as an oversight rather than intent

The username is normalized on the very next line, and the suite already pins both symmetric halves of this rule — test_duplicate_github_with_different_case_rejected (case folding matters) and test_duplicate_wallet_with_different_github_rejected (wallet dedup matters). Only the wallet × case combination was never covered.

Fix

A chain-aware _normalize_wallet_address(), applied at the three entry points (check_eligibility, claim_airdrop, _has_claimed):

address = (wallet_address or "").strip()
if chain in EVM_CHAINS and EVM_ADDRESS_RE.fullmatch(address):
    return address.lower()
return address

Solana is deliberately excluded: base58 addresses are genuinely case-sensitive, and folding them would merge distinct wallets — a worse bug than the one being fixed. The regex fullmatch also means a non-EVM-shaped string on base is left untouched rather than silently mangled.

Only the comparison is canonicalized; this is not a migration. Any pre-existing mixed-case rows stay as they are — happy to add a backfill if you want the historical rows collapsed too.

Tests — node/tests/test_airdrop_evm_wallet_case_double_claim.py

On current main:

FAILED test_same_base_wallet_different_checksum_case_rejected
FAILED test_same_base_wallet_uppercase_variant_rejected
FAILED test_has_claimed_matches_base_wallet_in_any_case
E   assert not True   # second claim on the same wallet succeeded
3 failed, 1 passed

With the fix: 4 passed.

The one that passes on main is test_solana_addresses_remain_case_sensitive, the guard for the exclusion above — it must pass on both sides, and does.

Regression: node/test_airdrop_v2.py, tests/test_airdrop_bridge_admin_auth.py, tests/test_airdrop_frontend_security.py73 passed.

Separate issue, not fixed here

While in this file: _determine_tier() derives total_prs from GET /search/commits?q=author:X merged:true — that's a commit count across all of GitHub, not merged PRs, and not scoped to the org RIP-305 specifies (GitHub returns 200 and treats merged:true as free text; author:torvaldstotal_count: 74054). So ~any user with 5 commits in their own repos maps to CORE = 200 wRTC. Left alone because the fix needs a product decision on repo/org scoping — flagging it rather than bundling it under this title. Happy to take it if you say how tiers should be scoped.

RTC payout address: RTCd1554f0f35576faf01d386a6be1c947f560dd0b7

…m twice

_has_claimed normalized the GitHub username but compared the wallet byte-exactly.
EVM addresses are case-insensitive (mixed case is only an EIP-55 display checksum
over the same 20 bytes), so the same Base wallet in two casings read as two
different wallets and the UNIQUE(github_username, wallet_address, chain) index did
not catch it either. N GitHub accounts could route N x 200 wRTC into one Base
wallet, defeating RIP-305's one-claim-per-wallet rule.

Add a chain-aware _normalize_wallet_address() applied in check_eligibility,
claim_airdrop and _has_claimed. Solana addresses are base58 and genuinely
case-sensitive, so they stay byte-exact.
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related tests Test suite changes size/M PR: 51-200 lines labels Jul 14, 2026
@Scottcjn

Copy link
Copy Markdown
Owner

Genuine anti-Sybil fix, but held pending completion: it only closes the hole on greenfield DBs. Any Base claim already stored in mixed/EIP-55 case stays exploitable (same wallet, new GitHub, different casing passes), and the fix silently changes stored addresses to lowercase. Add a backfill (UPDATE ... SET wallet_address = lower(...) WHERE chain='base') or case-insensitive compare for Base, plus a regression test that seeds a checksummed row then claims lowercase and asserts reject. Then it pays. — Sophia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) BCOS-L2 Beacon Certified Open Source tier BCOS-L2 (required for non-doc PRs) node Node server related size/M PR: 51-200 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants