code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-xeth/blob/d86fe0a9959c2b43c62716240d981ae95224e49e/src/CVXStaker.sol#L205

Vulnerability details

Impact

stakedBalance() maybe return wrong number,Causes AMO.sol not to work properly

Proof of Concept

stakedBalance() use for get the current staked balance of CVXStaker

The code is as follows:

    function stakedBalance() public view returns (uint256 balance) {
        balance = IBaseRewardPool(cvxPoolInfo.rewards).balanceOf(address(this));
    }

From the above code we can see that this method only considers the stake balance in convex and does not add the number of clpToken stored in CVXStaker contract If isCvxShutdown()==true, it will leave clpToken in the CVXStaker contract Or withdrawAllAndUnwrap() may also leave clpToken in the CVXStaker contract

Since these balances are not counted, in AMO.rebalanceUp(),removeLiquidity,removeLiquidityOnlyStETH may not work properly, because the balance may be sufficient, but still revert Take rebalanceUp() as an example:

    function rebalanceUp(
        RebalanceUpQuote memory quote
    )
....

        uint256 amoLpBal = cvxStaker.stakedBalance();

        if (quote.lpBurn > amoLpBal) revert LpBalanceTooLow();  //<-------@audit The balance is sufficient but cannot be executed because stakedBalance() is incorrect

Tools Used

Recommended Mitigation Steps

Modify stakedBalance() to add the clpToken that exists in the contract

    function stakedBalance() public view returns (uint256 balance) {
-       balance = IBaseRewardPool(cvxPoolInfo.rewards).balanceOf(address(this));
+       return clpToken.balanceOf(address(this)) + IBaseRewardPool(cvxPoolInfo.rewards).balanceOf(address(this));
    }

Assessed type

Context

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #33