hats-finance / Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb

0 stars 1 forks source link

`CvxRewardDistributor.initialize()` will always revert and contract can not be initialized #56

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xca916796af9d85793f73b7a856f04d53a5880c5856936c40804b610c6b738021 Severity: medium

Description: Description\

CvxRewardDistributor.initialize() is implemented as:


    function initialize(ICvx1 _cvx1) external initializer {
        address treasuryDao = cvgControlTower.treasuryDao();
@>        ICvxConvergenceLocker _cvxConvergenceLocker = cvgControlTower.cvxConvergenceLocker();
@>        ICvxStakingPositionManager _cvxStakingPositionManager = cvgControlTower.cvxStakingPositionManager();

        require(address(_cvxConvergenceLocker) != address(0), "CVX_LOCKER_ZERO");
        cvxConvergenceLocker = _cvxConvergenceLocker;

        require(address(_cvxStakingPositionManager) != address(0), "CVX_POSITION_MANAGER_ZERO");
        cvxStakingPositionManager = _cvxStakingPositionManager;

      . . . some code

    }

initialize() function is used to initialize the contract along with state variables.

The issue is with the value setting of cvxConvergenceLocker and cvxStakingPositionManager contract addresses. Both the contracts are core part of CvxRewardDistributor.sol contract and without it, the contracts various functionalities can not work which would affect the users of protocol.

Both contract address of cvxConvergenceLocker and cvxStakingPositionManager is actually fetched from cvgControlTower i.e from Convergence Control Tower contract which is one of core contract of Convergence protocol for various setter contract addresses.

cvgControlTower address used in CvxRewardDistributor.sol as:

    /// @dev Convergence control tower
    ICvgControlTowerV2 public constant cvgControlTower = ICvgControlTowerV2(0xB0Afc8363b8F36E0ccE5D54251e20720FfaeaeE7);

Reference contract- https://etherscan.io/address/0xB0Afc8363b8F36E0ccE5D54251e20720FfaeaeE7#readProxyContract

If you check the above address on Ethereum mainnet, it does not consist of addresses for both cvxConvergenceLocker and cvxStakingPositionManager, therefore the initialize function will revert or become unresponse or will show an unexpected behaviour. This is due to missing address variables being called from the Convergence Control Tower contract

Impact

Initialize() function will show unexpected behaviour or revert due to calling of unimplemented addresses i.e cvxConvergenceLocker and cvxStakingPositionManager from cvgControlTower. This will prevent initialization of CvxRewardDistributor contract and would affect the users functionalities.

Recommendation to fix\

Do not fetch the address of cvxConvergenceLocker and cvxStakingPositionManager from cvgControlTowersince it is implemented in cvgControlTower contract.

Pass the cvxConvergenceLocker and cvxStakingPositionManager value as address argument in initialize() function similar to _cvx1.

0xRizwan commented 4 months ago

correction in recommendation- ** since it is NOT implemented in cvgControlTower contract

0xRizwan commented 4 months ago

Similar issue instances: 1) CvgCvxStakingPositionService.initialize(), 2) CvxAssetStakerBuffer.initialize() and 3) CvxAssetStakingService.initialize()

PlamenTSV commented 4 months ago

The control tower is an upgradable contract. These contracts in scope are not yet lively deployed, you cannot look for them on etherscan. With deployment, the tower will be upgraded with the featured addresses.