Problem
Issue #463 ("deposit_handler::cancel_deposit (and withdrawal_handler::cancel_withdrawal) refund the recorded amount with no vault-balance invariant check, unlike execute_deposit's InsufficientVaultBalance guard") is closed, but cancel_deposit (contracts/deposit_handler/src/lib.rs, ~lines 530-580) is unchanged: it refunds the stored deposit.long_token_amount/deposit.short_token_amount directly, with no check against the vault's actual recorded or real balance:
let vault_client = DepositVaultClient::new(&env, &deposit_vault);
// Refund tokens
if deposit.long_token_amount > 0 {
vault_client.transfer_out(
&handler,
&deposit.initial_long_token,
&deposit.account,
&deposit.long_token_amount,
);
}
if deposit.short_token_amount > 0 {
vault_client.transfer_out( /* same shape */ );
}execute_deposit (in the same file) explicitly cross-checks the vault's actual recorded balance against the deposit's claimed contribution before moving funds and panics on a shortfall — cancel_deposit has no equivalent guard at all. withdrawal_handler::cancel_withdrawal has the identical shape (refunds the stored amount unconditionally, no balance check).
Why it matters
Both deposit_vault and withdrawal_vault hold pooled, commingled balances across every currently-pending record for a given token. Any scenario where the vault's actual balance for a token legitimately diverges from what a specific deposit/withdrawal record's stored amount claims (a fee-on-transfer token, or accounting drift from any of the other data_store/vault bugs already on file, such as #386/#542's record_transfer_in gaps) lets a single cancellation pay out more than that specific record ever contributed to the vault — funded silently out of other users' still-pending funds sitting in the same pooled vault. This is exactly the risk execute_deposit was hardened against, with cancel_deposit/cancel_withdrawal left with no equivalent hardening.
Scope
In scope
contracts/deposit_handler/src/lib.rs::cancel_depositcontracts/withdrawal_handler/src/lib.rs::cancel_withdrawal
Suggested fix
Apply the same style of balance check execute_deposit already uses before refunding in both cancel paths — cap the refund at (or panic on) the vault's actual/recorded balance for that token, or track balances per-record-key so a cancellation can never draw on funds it did not itself deposit.
Verification
cargo test -p deposit-handler
cargo test -p withdrawal-handler
Add a test where the vault's real balance for a token is artificially short of a pending record's claimed amount, and assert cancel_deposit/cancel_withdrawal reverts (or caps the refund) rather than silently drawing on other users' funds.
Problem
Issue #463 ("deposit_handler::cancel_deposit (and withdrawal_handler::cancel_withdrawal) refund the recorded amount with no vault-balance invariant check, unlike execute_deposit's InsufficientVaultBalance guard") is closed, but
cancel_deposit(contracts/deposit_handler/src/lib.rs, ~lines 530-580) is unchanged: it refunds the storeddeposit.long_token_amount/deposit.short_token_amountdirectly, with no check against the vault's actual recorded or real balance:execute_deposit(in the same file) explicitly cross-checks the vault's actual recorded balance against the deposit's claimed contribution before moving funds and panics on a shortfall —cancel_deposithas no equivalent guard at all.withdrawal_handler::cancel_withdrawalhas the identical shape (refunds the stored amount unconditionally, no balance check).Why it matters
Both
deposit_vaultandwithdrawal_vaulthold pooled, commingled balances across every currently-pending record for a given token. Any scenario where the vault's actual balance for a token legitimately diverges from what a specific deposit/withdrawal record's stored amount claims (a fee-on-transfer token, or accounting drift from any of the other data_store/vault bugs already on file, such as #386/#542'srecord_transfer_ingaps) lets a single cancellation pay out more than that specific record ever contributed to the vault — funded silently out of other users' still-pending funds sitting in the same pooled vault. This is exactly the riskexecute_depositwas hardened against, withcancel_deposit/cancel_withdrawalleft with no equivalent hardening.Scope
In scope
contracts/deposit_handler/src/lib.rs::cancel_depositcontracts/withdrawal_handler/src/lib.rs::cancel_withdrawalSuggested fix
Apply the same style of balance check
execute_depositalready uses before refunding in both cancel paths — cap the refund at (or panic on) the vault's actual/recorded balance for that token, or track balances per-record-key so a cancellation can never draw on funds it did not itself deposit.Verification
Add a test where the vault's real balance for a token is artificially short of a pending record's claimed amount, and assert
cancel_deposit/cancel_withdrawalreverts (or caps the refund) rather than silently drawing on other users' funds.