Skip to content

#104 — Lacks a Reentrancy Guard and (Parity with the Other Emergency Entrypoints) #885

Description

@Topmatrixmor2014

Labels: Official Campaign | FWC26 GrantFox OSS Maybe Rewarded contract soroban governance bug

This is a smart-contract issue for the GrantFox FWC26 campaign. Bring cancel_emergency_withdrawal in line with the other emergency entrypoints' guard patterns.

Requirements and Context

contracts/finchippay-contract/src/lib.rscancel_emergency_withdrawal (≈ line 1940):

require_not_paused(&env);
admin.require_auth();
let stored = get_admin(&env);
if admin != stored {
    panic!("Unauthorized");
}
  • It does not hold ReentrancyGuard::acquire (no external transfer, so lower risk — but inconsistent).
  • It does not call require_initialized (relies on get_admin which panics "Contract not initialized" anyway — acceptable).
  • It validates admin == get_admin (legacy single admin) while initiate_emergency_withdrawal/approve_emergency_withdrawal use the admin-signer set. Batch-3 feat(contract): add / to the Pauser role (currently only admin can pause) #58 flagged the privileged-entrypoint governance gap generally; this issue is the specific remaining asymmetry: a single legacy key can cancel any pending withdrawal that required N-of-M signer approvals, and can cancel others' withdrawals at will.

Objectives

  1. Add ReentrancyGuard::acquire(&env) to cancel_emergency_withdrawal for consistency.
  2. Decide and enforce the caller policy: require an admin signer (via get_admin_signers) OR document that the legacy admin may cancel; align with the initiate/approve caller model.
  3. Add tests for non-signer cancel attempts and guard parity.

Suggested Execution

  1. Fork and branch: git checkout -b fix/emergency-cancel-guards.
  2. Patch cancel_emergency_withdrawal in src/lib.rs.
  3. Add tests; run cargo fmt --check && cargo clippy -- -D warnings && cargo test.

Acceptance Criteria

  • cancel_emergency_withdrawal holds the reentrancy guard.
  • Caller policy matches the initiate/approve entrypoints (or is explicitly documented as legacy-only).
  • ≥3 tests; cargo test + wasm32v1-none build pass.

Guidelines

  • Do not weaken the existing emergency-withdrawal flow; document any policy change.

Timeframe: 24 hours

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions