Maintenance-pool `deposit()` allows pool-id squatting via `TokenMismatch` griefing

Aperta
  • Qualcosa non va?
#2 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@Wilfred007 ci sta già lavorando.

Dal 30/9/2026.

  • #430 di @Wilfred007 — aperta

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
20/100
Tipo di issue
Funzionalità
Chiarezza
Specificata chiaramente
Stato di attività
Ferma
Stack tecnologico
rust
Ambito
backend-api-design, security

Direzione di ricerca

Read contracts/maintenance-pool/src/lib.rs around deposit() and contracts/maintenance-pool/src/types.rs for the pool key, then inspect the pool ID generation assumption described in the issue. Review the open linked pull request before starting, since it may already be addressing this work. A complete fix must prevent token squatting, preserve permissionless recurring deposits and admin-authorized withdrawals, and include the described tests in contracts/maintenance-pool/src/test.rs; the issue specifies cargo test --workspace.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug GrantFox OSS Maybe Rewarded Official Campaign | FWC26 security Stellar Wave very hard

Background / Context

contracts/maintenance-pool/src/lib.rs::deposit() creates a MaintenancePool on first deposit for a given pool_id, locking in whatever token the first depositor supplies (pool.deposit_count > 0 && pool.token != token → TokenMismatch). pool_id is described in types.rs as "an off-chain-assigned id for a repo/org," implying it's likely derived deterministically (e.g., hash of owner/repo) and therefore guessable ahead of time by anyone watching GitHub activity.

Problem Statement

An attacker can call deposit(pool_id, attacker, some_worthless_token, 1) for any repo/org they expect to receive real sponsor funding, permanently locking that pool to the worthless token. Every subsequent legitimate sponsor deposit in the intended asset (e.g., USDC on Stellar) reverts with TokenMismatch, with no recovery path in the contract (no admin override, no per-token sub-pools). This is a cheap, repeatable denial-of-service against the "recurring maintenance funding" product surface, distinct from and worth separating from the escrow-id-squatting issue because the recovery constraints differ (a pool is meant to be long-lived and repeatedly topped up, so there's no natural "just use a different id" workaround the way there might be for a one-off escrow).

Requirements

  • Determine whether pool_id generation is/should be unpredictable (out of scope to change the backend, but document the assumption) or whether the contract must be hardened regardless.
  • Design and implement a fix: candidates include admin-gated pool creation (a separate create_pool(pool_id, token) admin call before deposit is permitted), an allowlist of valid tokens set at initialize, or a per-(pool_id, token) compound key so multiple tokens can coexist per pool without collision (requires rethinking get_pool/balance semantics).
  • Add tests in contracts/maintenance-pool/src/test.rs reproducing the squat and validating the fix.
  • Ensure the fix doesn't regress the "sponsors can deposit repeatedly over time" recurring-funding use case central to this contract's purpose.

Acceptance Criteria

  • Documented threat model for pool_id predictability
  • Implemented fix in contracts/maintenance-pool/src/lib.rs and types.rs as needed
  • Tests proving squat attempts no longer block legitimate sponsors
  • No breaking change to withdraw's admin-authorized payout flow
  • cargo test --workspace passes

Technical Notes / Hints

  • Code: contracts/maintenance-pool/src/lib.rs lines ~51-104, DataKey::Pool(u64) in types.rs.
  • Consider interaction with Deposit history keyed by (pool_id, deposit_index) if you change how pools are created/keyed.

Difficulty Justification

The naive fix (reject deposits from non-admin on pool creation) conflicts with the contract's explicit design goal of being sponsor-initiated and permissionless for deposits. A correct solution must preserve permissionless recurring deposits while eliminating the squatting vector — this is a genuine access-control/design tradeoff requiring judgment, not a mechanical patch.

Lingua principale
Rust
Stelle
1
Fork
102
Merge medio
4h 12m
PR unite (30g)
30

Preparare l'ambiente

  • Nessun Dockerfile né file Docker Compose
  • Ha un modello di pull request
  • Nessuna guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di MergeFi/contracts

Tutte le issue di MergeFi/contracts

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.