c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L170-L195

Vulnerability details

Impact

Last stakers may not receive funds back.

Proof of Concept

When user wants to withdraw, then he needs to initiate requestWithdraw. As some part of funds are locked as cvx token inside vlcvx that means that they should be withdrawn. When you lock cvx then amount is locked for some period, so you can't withdraw it for that time.

For such reason withdrawTime function exists, which purpose is to calculate time when needed amount will be available to withdraw.

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L170-L195

    function withdrawTime(
        uint256 _amount
    ) external view virtual override returns (uint256) {
        uint256 _priceInCvx = cvxPerVotium();
        (
            ,
            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();
    }

This function reverts in case if it didn't trigger return in the loop. So protocol developers doesn't expect that it's possible.

However, it is. In case if there are no items in the lockedBalances, but unlockable != 0, that means that all locks are expired and such situation is likely to occur when users are going to quit AfEth token, so there is small amount of funds locked and new locks are not created.

So imagine situation that unlockable is 10 eth and user would like to withdraw it, but because loop will be skipped, then function will revert, which means that user can't withdraw. It will be still possible to recover funds by relocking, but user will have to wait more.

Same problem exists in the VotiumStrategy.requestWithdraw function.

Tools Used

VsCode

Recommended Mitigation Steps

As i recommended in another finding you need to expect that totalLockedBalancePlusUnlockable at the beginning is already enough to fulfill request.

Assessed type

Error

c4-judge commented 9 months ago

0xleastwood marked the issue as duplicate of #63