milestones::allocate: issue_id uniqueness is only enforced within a single milestone_id — the same GitHub issue can be paid out from multiple milestones

Abierto
  • ¿Algo no está bien?
#51 4 comentarios 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@phalap1 ya está trabajando en esto.

Desde el 17/8/2026.

  • #192 de @phalap1 — abierto

Evaluación

Este issue todavía no se ha evaluado.

Descripción

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

Overview

milestones scopes an issue's allocation status by the pair (milestone_id, issue_id), not by issue_id alone: DataKey::IssueStatus(u64, u64) (contracts/milestones/src/types.rs:35), read/written exclusively as DataKey::IssueStatus(milestone_id, issue_id) throughout allocate and release_issue (contracts/milestones/src/lib.rs:121, 148, 179). allocate's only duplicate-allocation guard, milestone.allocations.contains_key(issue_id) (contracts/milestones/src/lib.rs:109), is likewise scoped to the one Milestone record it's operating on — it has no visibility into, and performs no check against, any other milestone's allocations.

Consequence: the same underlying GitHub issue_id can be allocated, and independently released, from multiple different milestones simultaneously, with the contract providing zero defense-in-depth against it. Concretely: allocate(milestone_id=1, issue_id=555, amount=X) and allocate(milestone_id=2, issue_id=555, amount=Y) both succeed unconditionally — nothing in either call inspects milestone 2's state while processing milestone 1's, or vice versa. Both allocations can subsequently be independently released via release_issue(1, 555, ...) and release_issue(2, 555, ...), each paying out its own full amount, for what is, off-chain, the same piece of merged work being paid for twice (or more) over.

This is a real gap even under the codebase's explicit "contracts are deliberately dumb, backend is the trusted oracle" trust model (README: "GitHub remains the source of truth... these contracts are deliberately dumb about that — they only know what the oracle tells them") — that model justifies the contract trusting the admin's judgment about whether work was done; it does not, by itself, justify providing zero structural defense against the admin being tricked, or making an honest mistake, into allocating the same issue_id twice across different milestones, something a single admin key operating many milestones over time is entirely capable of doing accidentally (e.g. a spreadsheet/database error on the backend side associating one GitHub issue with two different release milestones). Compare with escrow::fund, which enforces issue_id uniqueness globally via a single DataKey::Escrow(issue_id) key with no milestone-equivalent scoping concept at all — milestones' weaker, per-milestone-only scoping is a deliberate consequence of its data model (an issue_id legitimately needs to be nameable within many different milestones' allocations maps in principle), but the complete absence of any global "this issue_id is already claimed somewhere" registry is a gap escrow's design doesn't share.

Requirements

  • Add a global (contract-instance-wide) registry — e.g. DataKey::GlobalIssueClaim(issue_id) -> milestone_id — checked and set by allocate, rejecting a second allocate call for an issue_id that's already claimed by a different milestone_id, with a new Error variant distinguishing this from the existing per-milestone IssueAlreadyAllocated.
  • Decide what should happen when an issue's allocation is later removed (once the companion deallocate function from this batch's other issue lands) — the global claim should presumably be released at that point too, so the registry doesn't itself become a source of permanent false "already claimed" rejections.
  • Document, in the README's "Contracts" section for milestones, the intended guarantee explicitly: "an issue_id can be allocated in at most one milestone at a time."

Acceptance Criteria

  • Global issue-claim registry implemented and enforced in allocate
  • New Error variant distinguishing "already allocated in this milestone" from "already allocated in a different milestone"
  • Test: test_allocate_rejects_issue_already_claimed_by_different_milestone — allocate issue_id=X in milestone A, attempt to allocate the same issue_id in milestone B, assert rejection
  • Test: interaction with deallocate (if landed) — deallocating frees the global claim, allowing legitimate reallocation elsewhere
  • README updated with the explicit cross-milestone uniqueness guarantee
  • cargo test --workspace passes

Additional Notes

  • Precise references: contracts/milestones/src/types.rs:35 (IssueStatus(u64, u64) keying, confirming the scoping is milestone-relative, not global); contracts/milestones/src/lib.rs:109 (allocate's sole duplicate check, scoped to the single Milestone record already loaded into milestone — provably has no access to any other milestone's data within that function call).
  • Test sketch: test_double_funding_same_issue_across_two_milestones_pre_fix as a reproduction — create_milestone(1, ...), create_milestone(2, ...), allocate(1, 555, amount_a), allocate(2, 555, amount_b) (both currently succeed), release_issue(1, 555, recipients), release_issue(2, 555, recipients) (both currently succeed), assert the recipient(s) received two separate payouts for the same issue_id — demonstrating the double-payment is real and silent before the fix closes it.
  • Cross-references: #1 (escrow's issue-id squatting — a different mechanism, front-running by an unrelated party, not double-allocation by the legitimate admin across milestones); the companion cross-contract double-funding issue filed in this same batch (this issue's within-milestones-contract scope is the narrower, more directly fixable half of that broader concern); the "no deallocate/reallocate" issue filed in this batch (interacts directly with how a global claim registry should behave on deallocation).
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.