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

Users can't redeem vault's shares due to inverse logic in `VaultRouterBranch:redeem`

alexczm

Summary

Users can't redeem if the amount of assets to receive back is smaller than current lockedCreditCapacity

Vulnerability Details

Users who provided liquidity to vaults can redeem their share by calling initiateWithdrawal followed by redeem.
First, the available credit capacity and the locked credit capacity are cached. Then the actual redeem takes places where shares are burnt and assets are transferred from Vault to user, decreasing the vault's credit capacity.

Lastly the code should checks if there's enough credit capacity left in the vault.
If the before redeem credit capacity minus after redeem credit capacity is smaller than locked credit capacity, the redeem tx reverts:
if (beforeRedeemCC - CC <= lockedCcBeforeRedeem) then revert; this is equivalent to if (amountRedeemed <= lockedCcBeforeRedem) then revert.

function redeem(uint128 vaultId, uint128 withdrawalRequestId, uint256 minAssets) external {
...
// 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));// transfer asset to user, burn vault shares
...
// 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();
}
...
}

This means an user can't redeem an amount of shares worth less than locked credit capacity .
Let's take the following example:

  • value of assets deposited in vault : 100 usd

  • vault's debt : 20 usd

  • lockedCreditRatio : 0.4

    From these numbers we get :

  • creditCapacity (CC) : 80 usd

  • lockedCreditCapaciti (lockedCC) : 80 * 0.4 = 32 usd

    User want to redeem shares worth of 20 usd:

  • beforeCC : 80

  • lockedCC : 32

  • afterCC : 80 - 20 = 60

    If we add these value in the if condition :
    beforeCC - afterCC < lockedCC <=> 80 - 60 < 32 which is true and the transaction reverts.

Impact

Users can't redeem their shares unless the amount is large enough. The amount necessary for a successful redeem will vary depending on many aspects most users can't control resulting in a indefinitely redeem DoS.

Tools Used

Recommendations

Depending on what are the desired redeem conditions, different updates can be done.
If this comment is the desired behavior, then replace the lte with gte (greater than or equal) function.
But this will result in a redeem DoS for amounts bigger than locked credit capacity.

// 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.creditCapacityBeforeRedeemUsdX18.sub(vault.getTotalCreditCapacityUsd()).gte(
ctx.lockedCreditCapacityBeforeRedeemUsdX18.intoSD59x18()
)
) {
revert Errors.NotEnoughUnlockedCreditCapacity();
}

A better solution would be getLockedCreditCapacityUsd to not depend on vault's credit capacity, since it vary with each subsequent redeem, but only on vault's debt only. You want the vault a buffer on top of accumulated debt.
Consider the following pseudo-code;

function getLockedCreditCapacityUsd(Data storage self)
internal
view
returns (uint256 lockedCC)
int128 totalDebt = lockedCC(self);
if(totalDebt > 0) lockedCC = (totalDebt + bufferPercent * totalDebt);
// if totalDebt is negative (vault in credit) you still want to have a buffer
lockedCC = (- totalDebt);
​
return lockedCC;
}

By implementing this solution redeem doesn't affect locked credit capacity and connected markets have liquidity for any potential debt.

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 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.