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

Đang mở
  • Có gì không ổn?
#2 3 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@Wilfred007 đang làm issue này rồi.

Từ ngày 30/9/2026.

  • #430 của @Wilfred007 — đang mở

Đánh giá

Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức phù hợp với người mới
20/100
Loại issue
Tính năng
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Đình trệ
Công nghệ
rust
Lĩnh vực
backend-api-design, security

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
Rust
Star
1
Fork
102
Merge trung bình
4 giờ 12 phút
Pull request đã merge (30 ngày)
30

Chuẩn bị môi trường

  • Không có Dockerfile hay tệp Docker Compose
  • Có mẫu pull request
  • Không có hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của MergeFi/contracts

Tất cả issue của MergeFi/contracts

Issue tương tự

Thêm issue về Rust

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.