cvxCRVDepositor Manual Review Findings

CRD-01M: Overly Centralized Reward Lifecycle Management

Type	Severity	Location
Logical Fault		cvxCRVDepositor.sol:L39-L57

Description:

The reward lifecycle management of the cvxCRVDepositor contract is entirely centralized and permits the owner to arbitrarily transfer all assets under the management of the contract.

Example:

Copy
39function withdrawFromConvex(uint amount, bool claim) external onlyOwner {
40    rewards.withdraw(amount, claim);
41}
42
43function getRewardFromConvex(address[] calldata tokens, address[] calldata receivers) external onlyOwner {
44    rewards.getReward();
45    _withdrawToken(tokens, receivers);
46}
47
48function _withdrawToken(address[] calldata tokens, address[] calldata receivers) internal {
49    require(tokens.length == receivers.length, "recoveryToken: invalid data");
50    for (uint i; i < tokens.length; i++) {
51        IERC20(tokens[i]).safeTransfer(receivers[i], IERC20(tokens[i]).balanceOf(address(this)));
52    }
53}
54
55function withdrawToken(address[] calldata tokens, address[] calldata receivers) external onlyOwner {
56    _withdrawToken(tokens, receivers);
57}

Recommendation:

We advise this trait of the system to be re-evaluated and refactored to instead be performed trustlessly as currently there is no guarantee the tokens extracted or withdrawn from the reward contract will be redistributed to the correct parties.

Alleviation:

The Congruent team stated that the withdrawFromConvex operation will sit behind a timelock and multisignature wallet and that it exists in case some change in the Convex system occurs in the future that will require a migration of funds. Given that the ownership will remain somewhat centralized, we will consider this exhibit acknowledged.

CRD-02M: Deprecated Approval Paradigm

Type	Severity	Location
Standard Conformity		cvxCRVDepositor.sol:L33

Description:

The linked statements utilize the safeApprove function that has been officially deprecated.

Example:

Copy
30function deposit(uint amount) external {
31    cvxCRV.safeTransferFrom(msg.sender, address(this), amount);
32
33    cvxCRV.safeApprove(address(rewards), amount);
34    rewards.stake(amount);
35
36    cCRV.mint(msg.sender, amount);
37}

Recommendation:

We advise a direct approve or a safeIncreaseAllowance instruction to be utilized instead as the statements are meant to replace any previously set allowance to the new one.

Alleviation:

A direct approve function is now utilized in the code instead.