Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L181-L184 https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L74-L77

Vulnerability details

Bug Description

In AfEth.sol, whenever a user calls requestWithdraw() to queue a withdrawal, the time that they can withdraw is determined by withdrawTime():

AfEth.sol#L175-L176

    function requestWithdraw(uint256 _amount) external virtual {
        uint256 withdrawTimeBefore = withdrawTime(_amount);

withdrawTime() determines the withdrawal time as follows:

VotiumStrategy.sol#L174-L194

        (
            ,
            uint256 unlockable,
            ,
            ILockedCvx.LockedBalance[] memory lockedBalances
        ) = ILockedCvx(VLCVX_ADDRESS).lockedBalances(address(this));
        uint256 cvxAmount = (_amount * _priceInCvx) / 1e18;
        uint256 totalLockedBalancePlusUnlockable = unlockable +
            IERC20(CVX_ADDRESS).balanceOf(address(this));

        for (uint256 i = 0; i < lockedBalances.length; i++) {
            totalLockedBalancePlusUnlockable += lockedBalances[i].amount;
            // we found the epoch at which there is enough to unlock this position
            if (
                totalLockedBalancePlusUnlockable >=
                cvxUnlockObligations + cvxAmount
            ) {
                return lockedBalances[i].unlockTime;
            }
        }
        revert InvalidLockedAmount();

Where:

unlockable is the amount of CVX that can be unlocked currently.
lockedBalances stores the amount of CVX locked and unlockTime for each epoch that is still locked.

As seen from above, it iterates through lockedBalances to find an epoch where the amount of CVX unlockable is sufficient for the withdrawal. If an epoch is not found in the for-loop, withdrawTime() will revert.

The same logic is seen in the requestWithdraw() function of VotiumStrategy.sol:

VotiumStrategy.sol#L65-L102

        (
            ,
            uint256 unlockable,
            ,
            ILockedCvx.LockedBalance[] memory lockedBalances
        ) = ILockedCvx(VLCVX_ADDRESS).lockedBalances(address(this));
        uint256 cvxAmount = (_amount * _priceInCvx) / 1e18;
        cvxUnlockObligations += cvxAmount;

        uint256 totalLockedBalancePlusUnlockable = unlockable +
            IERC20(CVX_ADDRESS).balanceOf(address(this));

        for (uint256 i = 0; i < lockedBalances.length; i++) {
            totalLockedBalancePlusUnlockable += lockedBalances[i].amount;
            // we found the epoch at which there is enough to unlock this position
            if (totalLockedBalancePlusUnlockable >= cvxUnlockObligations) {
                // Some code that returns here...
            }
        }
        // should never get here
        revert InvalidLockedAmount();

However, this implementation does not check if totalLockedBalancePlusUnlockable contains sufficient CVX before iterating through lockedBalances. Therefore, even if the amount of CVX unlockable is sufficient, users will still be forced to wait until the next locked epoch to withdraw their funds.

Furthermore, if lockedBalances is empty, withdrawTime() and requestWithdraw() will always revert as the for-loop never runs. This could occur if deposit(), relock() and depositRewards() aren't called for a period of time, causing all CVX of the VotiumStrategy contract to be unlockable (ie. there are no epochs where VotiumStrategy still has CVX locked in the VLCVX contract).

Should this occur, the only way for users to queue withdrawals would be to call relock() to withdraw all unlockable CVX and lock them again. However, this would force users to wait the entire lock duration before they can withdraw, which is 16 weeks:

CvxLockerV2.sol#L1007

    // Duration that rewards are streamed over
    uint256 public constant rewardsDuration = 86400 * 7;

    // Duration of lock/earned penalty period
    uint256 public constant lockDuration = rewardsDuration * 16;

Impact

If the amount of unlockable CVX in VotiumStrategy is sufficient for withdrawals, users should be able to instantly withdraw their funds.

However, as withdrawTime() and requestWithdraw() do not check the amount of unlockable CVX first, users are instead forced to wait until the next lock epoch before being able to withdraw. This could even last up to 16 weeks.

Recommended Mitigation

In withdrawTime(), consider returning block.timestamp if totalLockedBalancePlusUnlockables is sufficient:

VotiumStrategy.sol#L181-L184

        uint256 totalLockedBalancePlusUnlockable = unlockable +
            IERC20(CVX_ADDRESS).balanceOf(address(this));

+       if (totalLockedBalancePlusUnlockable >= cvxUnlockObligations + cvxAmount) return block.timestamp;
        for (uint256 i = 0; i < lockedBalances.length; i++) {

The same change should be applied to requestWithdraw() as well:

VotiumStrategy.sol#L74-L77

        uint256 totalLockedBalancePlusUnlockable = unlockable +
            IERC20(CVX_ADDRESS).balanceOf(address(this));

+       if (totalLockedBalancePlusUnlockable >= cvxUnlockObligations) {
+           withdrawIdToWithdrawRequestInfo[latestWithdrawId] = WithdrawRequestInfo({
+               cvxOwed: cvxAmount,
+               withdrawn: false,
+               epoch: withdrawEpoch,
+               owner: msg.sender
+           });
+           return latestWithdrawId;
+       }
        for (uint256 i = 0; i < lockedBalances.length; i++) {

Assessed type

Other

code-423n4 / 2023-09-asymmetry-findings