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
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.
Background / Context
contracts/maintenance-pool/src/lib.rs::deposit()creates aMaintenancePoolon first deposit for a givenpool_id, locking in whatevertokenthe first depositor supplies (pool.deposit_count > 0 && pool.token != token→TokenMismatch).pool_idis described intypes.rsas "an off-chain-assigned id for a repo/org," implying it's likely derived deterministically (e.g., hash ofowner/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 withTokenMismatch, with no recovery path in the contract (noadminoverride, 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
pool_idgeneration is/should be unpredictable (out of scope to change the backend, but document the assumption) or whether the contract must be hardened regardless.create_pool(pool_id, token)admin call beforedepositis permitted), an allowlist of valid tokens set atinitialize, or a per-(pool_id, token)compound key so multiple tokens can coexist per pool without collision (requires rethinkingget_pool/balancesemantics).contracts/maintenance-pool/src/test.rsreproducing the squat and validating the fix.Acceptance Criteria
pool_idpredictabilitycontracts/maintenance-pool/src/lib.rsandtypes.rsas neededwithdraw's admin-authorized payout flowcargo test --workspacepassesTechnical Notes / Hints
contracts/maintenance-pool/src/lib.rslines ~51-104,DataKey::Pool(u64)intypes.rs.Deposithistory 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.