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

The `VaultRouterBranch::redeem` function doesn't revert when the credit capacity delta is greater than the locked credit capacity before the state transition

bube

Summary

The VaultRouterBranch::redeem function doesn't revert when ctx.creditCapacityBeforeRedeemUsdX18.sub(vault.getTotalCreditCapacityUsd()) is greater than lockedCreditCapacityBeforeRedeemUsdX18.

Vulnerability Details

The VaultRouterBranch::redeem function redeems a given amount of index tokens in exchange for collateral assets from the provided vault, after the withdrawal delay period has elapsed:

​
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());
}
​

According to the comment in the function if the credit capacity delta is greater than the locked credit capacity before the state transition, the function should revert. The problem is that the function does the opposite. The function checks if the credit capacity delta is less than or equal the locked credit capacity before redeem and if it is true, the function reverts. This contradicts the comment and allows the function to continue with its execution in case when the credit capacity delta is greater than the lockedCreditCapacityBeforeRedeemUsdX18.

Impact

The function will not revert when the credit capacity delta is greater than the locked credit capacity before the state transition. This means there will be not enough unlocked credit capacity.

Tools Used

Manual Review

Recommendations

Change the if statement with the following:

​
+ if (
+ ctx.creditCapacityBeforeRedeemUsdX18.sub(vault.getTotalCreditCapacityUsd()).gt(
+ ctx.lockedCreditCapacityBeforeRedeemUsdX18.intoSD59x18()
+ )
+ ) {
+ revert Errors.NotEnoughUnlockedCreditCapacity();
+ }
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

The check in VaultRouterBranch::redeem should be comparing remaining capacity against required locked capacity not delta against locked capacity

Support

FAQs

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

Give us feedback!