Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unprotected early withdrawal risk through unchecked vault state in VaultRouterBranch::redeem

sabanaku77

Summary

The redeem function in VaultRouterBranch.sol has a critical security vulnerability where it fails to check the vault's locked status, emergency pause conditions, and global withdrawal limits before allowing withdrawals. This flaw could enable unauthorized withdrawals in situations where the protocol should restrict them, potentially impacting financial stability and security.

Finding Description

486: function redeem(uint128 vaultId, uint128 withdrawalRequestId, uint256 minAssets) external {
// fetch storage slot for vault by id
Vault.Data storage vault = Vault.loadLive(vaultId);
​
// load storage slot for previously created withdrawal request
WithdrawalRequest.Data storage withdrawalRequest =
WithdrawalRequest.loadExisting(vaultId, msg.sender, withdrawalRequestId);
​
// revert if withdrawal request already fulfilled
if (withdrawalRequest.fulfilled) revert Errors.WithdrawalRequestAlreadyFulfilled();
​
// revert if withdrawal request delay not yet passed
if (withdrawalRequest.timestamp + vault.withdrawalDelay > block.timestamp) {
revert Errors.WithdrawDelayNotPassed();
}
​
// prepare the `Vault::recalculateVaultsCreditCapacity` call
uint256[] memory vaultsIds = new uint256[](1);
vaultsIds[0] = uint256(vaultId);
​
// updates the vault's credit capacity before redeeming
Vault.recalculateVaultsCreditCapacity(vaultsIds);
​
// define context struct, get withdraw shares and associated assets
RedeemContext memory ctx;
ctx.shares = withdrawalRequest.shares;
ctx.expectedAssetsX18 = getIndexTokenSwapRate(vaultId, ctx.shares, false);
​
// load the mm engine configuration from storage
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
​
// cache vault's redeem fee
ctx.redeemFee = vault.redeemFee;
​
// get assets minus redeem fee
ctx.expectedAssetsMinusRedeemFeeX18 =
ctx.expectedAssetsX18.sub(ctx.expectedAssetsX18.mul(ud60x18(ctx.redeemFee)));
​
// calculate assets minus redeem fee as shares
ctx.sharesMinusRedeemFeesX18 =
getVaultAssetSwapRate(vaultId, ctx.expectedAssetsMinusRedeemFeeX18.intoUint256(), false);
​
// get the shares to send to the vault deposit and redeem fee recipient
ctx.sharesFees = ctx.shares - ctx.sharesMinusRedeemFeesX18.intoUint256();
​
// cache the vault's credit capacity before redeeming
ctx.creditCapacityBeforeRedeemUsdX18 = vault.getTotalCreditCapacityUsd();
​
// cache the locked credit capacity before redeeming
ctx.lockedCreditCapacityBeforeRedeemUsdX18 = vault.getLockedCreditCapacityUsd();
​
// redeem shares previously transferred to the contract at `initiateWithdrawal` and store the returned assets
address indexToken = vault.indexToken;
uint256 assets =
IERC4626(indexToken).redeem(ctx.sharesMinusRedeemFeesX18.intoUint256(), msg.sender, address(this));
​
// get the redeem fee
if (ctx.sharesFees > 0) {
IERC4626(indexToken).redeem(
ctx.sharesFees, marketMakingEngineConfiguration.vaultDepositAndRedeemFeeRecipient, address(this)
);
}
​
// require at least min assets amount returned
if (assets < minAssets) revert Errors.SlippageCheckFailed(minAssets, assets);
​
// invariant: received assets must be > 0 even when minAssets = 0
if (assets == 0) revert Errors.RedeemMustReceiveAssets();
​
// if the credit capacity delta is greater than the locked credit capacity before the state transition, revert
if (
ctx.creditCapacityBeforeRedeemUsdX18.sub(vault.getTotalCreditCapacityUsd()).lte(
ctx.lockedCreditCapacityBeforeRedeemUsdX18.intoSD59x18()
)
) {
revert Errors.NotEnoughUnlockedCreditCapacity();
}
​
// set withdrawal request to fulfilled
withdrawalRequest.fulfilled = true;
​
// emit an event
emit LogRedeem(vaultId, msg.sender, ctx.sharesMinusRedeemFeesX18.intoUint256());
}

The function lacks crucial validations:

  • It does not check if the vault is locked due to security or operational reasons.

  • The function does not verify if the protocol is in an emergency pause state, which should prevent any withdrawals.

  • There is no enforcement of protocol-wide withdrawal caps to prevent excessive fund outflows in a short period.

  • This function only verifies the delay but does not check whether the vault is locked, the protocol is paused, or global withdrawal limits have been reached, leading to potential security risks.

Impact

  • Users could withdraw funds from locked vaults or during emergency protocol suspensions.

  • Without a withdrawal cap, the function allows unrestricted withdrawals, potentially causing liquidity issues and destabilizing the protocol.

  • In the event of a governance decision to pause withdrawals, malicious users could still execute withdrawals, undermining protocol integrity.

Likelihood

  • The redeem function is externally callable, meaning any user can attempt a withdrawal.

  • Since no additional validation checks are implemented, a user can withdraw funds as long as the withdrawal delay has passed.

  • This flaw affects all vaults using this contract, making it a systemic risk.

Recommendation

  • Ensure withdrawals are blocked if the vault is locked:

if (vault.isLocked()) {
revert Errors.VaultLocked();
}

  • Block withdrawals when the protocol is in an emergency pause state:

if (protocol.isPaused()) {
revert Errors.ProtocolPaused();
}

  • Restrict withdrawals if the daily cap has been reached:

uint256 amount = calculateWithdrawalAmount(vaultId, withdrawalRequestId);
if (protocol.withdrawalsToday() + amount > protocol.maxDailyWithdrawals()) {
revert Errors.WithdrawalLimitExceeded();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!