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

Abierto
  • ¿Algo no está bien?
#2 3 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

@Wilfred007 ya está trabajando en esto.

Desde el 30/9/2026.

  • #430 de @Wilfred007 — abierto

Evaluación

Dificultad
5/5
Tiempo estimado
Más de una semana
Aptitud para principiantes
20/100
Tipo de issue
Nueva funcionalidad
Claridad
Bien especificado
Estado de actividad
Estancado
Stack tecnológico
rust
Área
backend-api-design, security

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Rust
Estrellas
1
Forks
102
Merge medio
4 h 12 min
PR fusionados (30 d)
30

Preparar el entorno

  • Sin Dockerfile ni archivo de Docker Compose
  • Tiene una plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/contracts

Todos los issues de MergeFi/contracts

Issues similares

Más issues de Rust

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.