Skip to content

Restrict vault distribute to untracked surplus so the admin cannot move depositor-backed USDC without the sweep timelock #1392

Description

@greatest0fallt1me

Summary

CalloraVault::distribute is documented as "Admin distribute USDC surplus". In fact it only checks that the vault's on-ledger USDC balance is at least amount. The admin can therefore transfer funds that back depositors' tracked balance, at once, while paused, with no timelock and no admin cooldown. That bypasses every protection that propose_sweep / execute_sweep enforce for the same operation. After such a call, balance() reports funds that the vault no longer holds, and later withdraw / deduct calls fail at the token transfer.

Background and evidence

contracts/vault/src/lib.rs, distribute (lines 1175–1207):

let on_ledger = usdc.balance(&env.current_contract_address());
if on_ledger < amount {
    panic!("insufficient USDC balance");
}
usdc.transfer(&env.current_contract_address(), &to, &amount);
  • No surplus check. Unlike propose_sweep (around line 2163) and execute_sweep (around line 2252), it does not compute surplus = on_ledger - DataKey::Balance.
  • No timelock. It does not go through timelock or the admin::guard cooldown that execute_sweep uses (admin::guard(&env, Symbol::new(&env, "sweep"))).
  • Panics instead of errors. It returns () and panics, with .unwrap() on require_admin and require_positive_amount and expect("USDC Token not set"). Integrators get opaque traps.
  • Unreachable code. There is a redundant if amount <= 0 { panic!(..) } after require_positive_amount(amount).unwrap().
  • Surplus-only paths already exist. admin_rescue (around line 2753) already protects the tracked balance by passing protected_balance to rescue::rescue_funds. distribute is now the only admin path that can break the invariant usdc.balance(vault) >= DataKey::Balance.
  • A test depends on the current behaviour. test_value_conservation.rs::execute_sweep_fails_when_surplus_drops_below_amount_after_proposal calls client.distribute(&owner, &other, &10) to drain surplus, and it is currently #[ignore]d as stale.

Scope

In scope

  1. Change the signature to pub fn distribute(env: Env, caller: Address, to: Address, amount: i128) -> Result<(), VaultError>.
  2. Enforce:
    • admin auth;
    • amount > 0;
    • to != current_contract_address() and to != usdc_token, mirroring withdraw_to;
    • amount <= on_ledger - tracked, computed with checked_sub, else InsufficientBalance.
  3. Apply the same admin cooldown as sweep, via admin::guard(&env, Symbol::new(&env, "distribute")), or remove distribute in favour of propose_sweep. Pick one and justify it in the PR. Whichever you pick, no admin path may move tracked funds.
  4. Keep it allowed when paused (recovery), but only for surplus.
  5. Event. Keep the distribute topic shape. Add caller to the topics so indexers can attribute the action, and update docs/EVENT_SCHEMA.md and tests/event_topic_catalog.rs if the shape changes.
  6. Fix the ignored test. Rewrite execute_sweep_fails_when_surplus_drops_below_amount_after_proposal and remove its #[ignore].

Out of scope

  • Changing execute_sweep or admin_rescue semantics.
  • Multi-token surplus accounting.

Design constraints

  • Invariant: after any successful admin call, usdc.balance(vault) >= balance(). Add it to docs/INVARIANTS.md.
  • Error precedence: auth, then amount, then recipient, then config, then surplus.
  • No unwrap, expect, panic! or assert! may remain in distribute.
  • Size: the WASM size delta must be at most 1%.

Test plan

# Scenario Expected
1 Tracked 100, on-ledger 140, distribute 40 Ok; recipient +40; balance() == 100
2 Tracked 100, on-ledger 140, distribute 41 InsufficientBalance; no transfer
3 Tracked 100, on-ledger 100, distribute 1 InsufficientBalance (on main today this succeeds and breaks backing)
4 Non-admin caller Unauthorized
5 amount of 0 or −1 AmountNotPositive
6 to is the vault or the USDC token typed recipient error
7 While paused, surplus only succeeds
8 Cooldown active, if you chose the cooldown the cooldown error variant; no transfer
9 Proptest: random deposits, deducts and distributes the backing invariant always holds

Regression proof. Row 3 succeeds on main. Afterwards usdc.balance(vault) < balance() and withdraw(balance()) traps. Show this in the PR.

Acceptance criteria

  • distribute returns Result<(), VaultError> and cannot move tracked funds.
  • grep -nE 'unwrap\(|expect\(|panic!|assert!' contracts/vault/src/lib.rs shows no matches inside distribute.
  • Every row of the test plan is covered. cargo test -p callora-vault distribute passes, and the previously ignored sweep test now runs.
  • cargo fmt --all -- --check and cargo test --workspace pass, and CI is green.
  • docs/INVARIANTS.md and docs/EVENT_SCHEMA.md are updated.
  • Coverage of distribute is at least 95%.

Getting started

git checkout -b fix/vault-distribute-surplus-only
cargo test -p callora-vault test_value_conservation -- --include-ignored
cargo build --release --target wasm32v1-none -p callora-vault

Compare distribute, propose_sweep, execute_sweep and admin_rescue in contracts/vault/src/lib.rs and rescue_funds in contracts/vault/src/rescue.rs.

PR requirements

  • Link this issue (Closes #<n>).
  • Explain the threat (the admin draining backed funds) and how the change closes it.
  • Paste the regression output from main and the passing output after your change.
  • No unrelated refactors of the sweep code.
  • Example commit: fix(vault): limit distribute to untracked surplus.

Priority rationale

High. Today one admin key can make the deployed vault insolvent relative to its own accounting, and no timelock warns depositors. The fix is security-critical and touches auth, the invariants and the pause policy.

Activity

  1. abiolaayanfeoluwa93-rgb commented on Oct 18, 2026

    @abiolaayanfeoluwa93-rgb

    @abiolaayanfeoluwa93-rgb has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    Hi, maintainer. I’m available to work on this issue and confident that I can approach it carefully while meeting the project’s expectations. Kindly consider me for the assignment.

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @abiolaayanfeoluwa93-rgb to this issue.

  2. nugarjr5 commented on Oct 18, 2026

    @nugarjr5
    Contributor

    @nugarjr5 has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    I believe I can do this 💯

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @nugarjr5 to this issue.

  3. Jerry-Tekh commented on Oct 19, 2026

    @Jerry-Tekh
    Contributor

    @Jerry-Tekh has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    good morning maintainer. please let me work on this issue . i will implement what is needed as required.

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @Jerry-Tekh to this issue.

  4. drips-wave commented on Oct 19, 2026

    @drips-wave

    Congratulations, @abiolaayanfeoluwa93-rgb! 🎉 Your application was accepted by the repo's maintainers, and the issue is due on October 22, 2026.

    🧑‍💻 @abiolaayanfeoluwa93-rgb: Please resolve the issue such that the repo's maintainers have enough time to review your contribution before the due date. You'll earn Points for completing the issue on-time, which will make you eligible for a share of the Stellar Wave Program's reward pool.

    Warning

    When opening a PR, please link it to this issue to ensure it gets tracked accurately. Points are awarded when this issue is marked as completed by the maintainer.

    🤠 Repo maintainers: Please keep an eye on the contributor's progress and review their work before the due date. You can manage this issue, including adjusting its complexity and points, here.

    🌊 Happy Wave 🌊

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

Metadata

Metadata

Labels

Stellar WaveStellar Wave Program issuepriority:highHigh implementation difficultysecuritySecurity hardeningsmart-contractSoroban smart-contract workvaultVault contract

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions