Skip to content

deposit_handler/withdrawal_handler: closed issue #461's global_pause_key bypass via direct handler calls is still unfixed #549

Description

@abayomicornelius

Problem

Issue #461 ("exchange_router: global protocol pause (global_pause_key) is enforced only in router wrappers, never inside deposit_handler/withdrawal_handler themselves") is closed, but global_pause_key is still referenced in exactly one file in the entire contracts tree:

grep -rn "global_pause_key" --include="*.rs" contracts/ | grep -v exchange_router
# (no output)

deposit_handler and withdrawal_handler each gained a market-specific pause check since #461 was filed (is_market_paused_key, from issue #366 — the oracle circuit-breaker pause), but neither ever checks global_pause_key, the flag exchange_router::set_paused/schedule_unpause/execute_unpause actually manage:

// contracts/deposit_handler/src/lib.rs, create_deposit / execute_deposit
if ds.get_bool(&is_market_paused_key(&env, &params.market)) {
    panic_with_error!(&env, Error::MarketPaused);
}
// no check anywhere against global_pause_key

// contracts/withdrawal_handler/src/lib.rs — identical shape

Why it matters

create_deposit/execute_deposit/create_withdrawal/execute_withdrawal remain public entrypoints on the handler contracts themselves. Anyone (or any keeper) can call them directly, completely bypassing exchange_router, and therefore bypassing the protocol's manual emergency pause entirely — the exact bypass #461 described. An admin who calls exchange_router::set_paused(true) during an incident has stopped only the router's own wrappers; a caller who knows the handler addresses (which are public, since they're stored in exchange_router's own instance storage and used throughout the test suite) can keep depositing and withdrawing on every market for the full duration of the declared pause. There is also still no router wrapper for execute_deposit/execute_withdrawal at all (the keeper always calls the handler directly by design), so a keeper can keep executing already-pending deposits/withdrawals throughout a pause regardless of this fix.

Scope

In scope

  • contracts/deposit_handler/src/lib.rscreate_deposit and execute_deposit.
  • contracts/withdrawal_handler/src/lib.rscreate_withdrawal and execute_withdrawal.

Out of scope

Suggested fix

Add a global_pause_key check inside deposit_handler/withdrawal_handler themselves (alongside the existing is_market_paused_key check), not only in exchange_router's wrappers, so the emergency pause holds regardless of which entrypoint a caller uses.

Verification

cargo test -p deposit-handler
cargo test -p withdrawal-handler

Add a test that sets global_pause_key to true directly in data_store (bypassing exchange_router), then calls deposit_handler::create_deposit/withdrawal_handler::create_withdrawal directly, and asserts both revert.

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