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

オープン
#51 コメント 4 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@phalap1 がすでに取り組んでいます。

2026年8月17日 から。

  • #192 @phalap1 による — オープン

評価

この issue はまだ評価されていません。

説明

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).
主要言語
Rust
スター
1
フォーク
102
平均マージ
4時間 12分
マージ済み PR(30日)
30

環境構築

  • Dockerfile・Docker Compose ファイルなし
  • プルリクエストのテンプレートあり
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/contracts のほかの issue

MergeFi/contracts の issue をすべて見る

似ている issue

Rust の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。