Convex staking contracts authenticity can not be determined due to lack of setter functionality

[hats-bug-reporter[bot]]

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

Description: Description\ CvxAssetStakingService.sol and CvgCvxStakingPositionService.sol are the contracts which are in charge of registering deposits and withdraws of staking position. Both of these contracts inherits from the StakingServiceBase which implements all the staking logic. Any user can own a staking position by depositing ETH, CVX or cvgCVX tokens depending on both contracts.

Staking positions are represented by a unique NFT. When the user deposits these listed tokens, the contracts issues a unique position NFTs to depositor i.e msg.sender

For minting the staking position NFT, both contracts calls CvxStakingPositionManager.mint() function in their deposit functions.

This can be checked here in CvgCvxStakingPositionService.sol and in CvxAssetStakingService.sol, it can be checked here

For more details, Please check #43

CvxStakingPositionManager.mint() can only be accessed by Convex StakingContract and #43 correctly highlights the issue. Please see the recommendation.

The issue here is that, Both CvxAssetStakingService.sol and CvgCvxStakingPositionService.sol are CVX staking contract can not be authenticated by contract owner via below mapping:

    /// @dev Determines if an address is a cvx staking contract and therefore can trigger withdrawals from the CvxBlackHole.
    mapping(address => bool) public isCvxStaking; /// contractAddress => bool

This is due to toggleCVXStakingContract() function is missing in contract so owner can not set both CVX contract as part of isCvxStaking mapping.

For isStakingContract address determination, CvgControlTowerV2.sol contract had used toggleStakingContract() function to make the staking contracts a part of isStakingContract mapping so they function caller can be validated for such contracts.

However, such toggle functionality is missing in CvxStakingPositionManager contract and must be added.

Impact

The CVX staking contracts can not be validated as there is no toggle functionality to make both CVX contracts to be part of isCvxStaking mapping. This would break the intended design and protocol functionality in validating the CVX staking contracts. In short, CVX staking contracts can not be validated in current implementation.

Recommendation to fix

Add toggleCVXStakingContract() in CvxStakingPositionManager.sol contract which is similar to toggleStakingContract() function.

    /// @dev Determines if an address is a cvx staking contract and therefore can trigger withdrawals from the CvxBlackHole.
    mapping(address => bool) public isCvxStaking; /// contractAddress => bool

     . . . some code

+    /**
+     * @notice Toggle a CVX staking contract, can only be called by the owner.
+     * @param contractAddress address of the CVX staking contract to toggle
+     */
+    function toggleCVXStakingContract(address contractAddress) external onlyOwner {
+        isCvxStaking[contractAddress] = !isStakingContract[contractAddress];
    }

[0xRizwan]

Please ignore this issue and check #59

[PlamenTSV]

isCvxStaking is unused, currently the control tower is being queried directly: require(cvgControlTower.isStakingContract(msg.sender), "NOT_STAKING")