Skip to content

Comprehensive access-control boundary audit across all entrypoints, including the permissionless-after-deadline refund path #30

Description

@chonilius

Background / Context

Across the three contracts, every entrypoint falls into one of three access categories: sponsor-authorized (fund, create_milestone, deposit — via sponsor.require_auth()), admin-authorized (release, allocate, release_issue, cancel_milestone, withdraw, and refund's early path — via require_admin(&env)?.require_auth()), and genuinely permissionless (refund's post-deadline path, deliberately callable by "anyone" per the README: "refund is sponsor-protective, so it deliberately doesn't require the sponsor's own signature," plus all get_* view functions). This task is a systematic, function-by-function audit verifying every entrypoint's actual enforced access category matches its intended one exactly, plus a deep look at the specific risks of the one deliberately-permissionless state-changing function.

Problem Statement

Two distinct threads: (1) A full audit table — for every public function across all three contracts, what access control is actually enforced by the code (not just the doc comment's claim) — to catch any drift between intention and implementation (the cancel_milestone/release_issue closed-check asymmetry tracked in a separate issue is exactly the kind of drift this audit should have caught systematically). (2) A focused deep-dive on refund's permissionless-after-deadline path specifically, since it's the only state-changing function in the entire system callable by a completely arbitrary, unauthenticated caller: does calling refund on behalf of someone else cost the caller anything (they pay the transaction's resource fees on Stellar to help a stranger get their refund — is this an economically rational thing for "anyone" to actually do, or does the permissionless design implicitly assume mergefi-backend itself will be the one calling it, making "anyone can call it" more of a liveness-fallback guarantee than a realistic expectation of altruistic third parties, and if so is that fallback actually load-bearing or cosmetic)? Could a malicious "anyone" grief a sponsor by calling refund at the earliest possible moment after deadline in a way that's harmful to the sponsor (e.g., if the sponsor would have preferred the funds stay escrowed slightly longer in hopes of last-minute merge, is there any way for them to signal/enforce that, or does any passerby's post-deadline refund call unconditionally override the sponsor's implicit preference with no recourse)?

Requirements

  • Produce a complete access-control audit table (function, contract, intended access level per doc comments/README, actually-enforced access level per code, match/mismatch verdict) for all ~25 public functions across the three contracts.
  • Fix any mismatches found (cross-reference already-tracked issues like the cancel_milestone one rather than duplicating; file precise new issues or fix directly for anything not already covered).
  • Deliver the focused analysis on refund's permissionless path: whether it's genuinely load-bearing given realistic caller incentives, and whether the sponsor has any way to express a preference against early third-party-triggered refund once the deadline passes (if not, and if that's judged a real gap, propose and implement a design — e.g., a sponsor-settable "auto-extend" or "grace period" flag set at fund time — balancing against not undermining the sponsor-protective guarantee refund's permissionlessness exists to provide).
  • Add tests covering every entry in the audit table's access-control claim (a non-admin caller cannot call admin-gated functions, a non-sponsor caller cannot call sponsor-gated functions on someone else's behalf, refund's permissionless path genuinely works with an arbitrary unrelated caller both before... no, specifically only after deadline).

Acceptance Criteria

  • Complete access-control audit table delivered covering every public function in all three contracts
  • Any found mismatches fixed or precisely cross-referenced/filed
  • Written analysis of refund's permissionless-path economics and griefing surface
  • Design decision (implemented, or explicitly justified as unnecessary) on sponsor ability to influence post-deadline refund timing
  • Access-control test matrix added covering every function's enforced boundary
  • cargo test --workspace passes

Technical Notes / Hints

  • require_admin is defined identically (structurally) three times, once per contract (escrow/src/lib.rs:256-261, milestones/src/lib.rs:277-282, maintenance-pool/src/lib.rs:168-173) — good candidate to also fold into the shared-crate extraction work if that's landing concurrently, but not required for this issue's scope.
  • The refund function's full logic is contracts/escrow/src/lib.rs:137-170 — note it has no equivalent in milestones or maintenance-pool (neither has a "anyone can trigger after a deadline" concept at all — milestones' cancel_milestone and maintenance-pool's withdraw are both admin-only with no timeout escape hatch), which is itself worth explicitly noting as an inconsistency across the three contracts' sponsor-protection guarantees worth deciding whether to address.

Difficulty Justification

Requires disciplined, exhaustive systematic auditing (easy to shortcut and miss exactly the kind of subtle asymmetry that's already caused a real bug elsewhere in this codebase) combined with genuine economic/game-theoretic reasoning about a permissionless-caller design pattern that's easy to wave away as "fine, it's just a fallback" without actually working through whether the fallback is realistically load-bearing or a false sense of security.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26securitySecurity-related issuetestingTesting/QA infrastructurevery hardVery difficult task, expert-level effort required

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions