c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Bug Description

In VotiumStrategy.sol, the relock() function is used to withdraw all unlockable CVX and then lock an appropriate amount of CVX again.

It does so by calling lock() of the VLCVX contract:

VotiumStrategy.sol#L145-L148

        if (cvxAmountToRelock > 0) {
            IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmountToRelock);
            ILockedCvx(VLCVX_ADDRESS).lock(address(this), cvxAmountToRelock, 0);
        }

Where:

cvxAmountToRelock is the total amount of CVX withdrawn - the amount of CVX used for pending withdrawals.

In the VLCVX contract, it is possible for lock() to revert if isShutdown is set to true:

CvxLockerV2.sol#L1465

    function _lock(address _account, uint256 _amount, uint256 _spendRatio, bool _isRelock) internal {
        require(_amount > 0, "Cannot stake 0");
        require(_spendRatio <= maximumBoostPayment, "over max spend");
        require(!isShutdown, "shutdown");

Note that the contract's owner can call shutdown() to set isShutdown to true.

As such, if isShutdown is set to true, relock() will always revert as it does not check the value of isShutdown before making a call to lock().

relock() is called by the withdraw() function:

VotiumStrategy.sol#L109-L117

    function withdraw(uint256 _withdrawId) external override {
        // Some checks here...

        relock();

Therefore, if VLCVX's owner sets isShutdown to true, the withdraw() function will permanently be DOSed as relock() will always revert.

Impact

Should VLCVX's owner decide to shutdown the contract, the relock() function in VotiumStrategy.sol will always revert. As a result, withdrawals will permanently be DOSed as withdraw() in VotiumStrategy.sol and AfEth.sol call relock() in their logic.

This leads to a loss of funds for users as they will no longer be able to withdraw their ETH using the withdraw() function.

Additionally, since relock() is the only function that withdraws unlockable CVX from the VLCVX contract, all CVX will forever be stuck in as there is no other way to withdraw the locked CVX.

Recommended Mitigation

In relock(), consider calling lock() only when isShutdown is false:

VotiumStrategy.sol#L145-L148

-       if (cvxAmountToRelock > 0) {
+       if (cvxAmountToRelock > 0 && !ILockedCvx(VLCVX_ADDRESS).isShutdown) {
            IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmountToRelock);
            ILockedCvx(VLCVX_ADDRESS).lock(address(this), cvxAmountToRelock, 0);
        }

Assessed type

DoS

elmutt commented 1 year ago

https://github.com/asymmetryfinance/afeth/pull/164

c4-judge commented 1 year ago

0xleastwood marked the issue as duplicate of #50