Vault.claimRewards can break if Convex changes the operator

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/Vault.sol#L183

Vulnerability details

Impact

Vault.claimRewards assumes that CVX will always get minted to the Vault (if there is a CRV reward). If CVX does not get minted, the claim will fail, preventing payout of CRV and extra rewards. Looking at the Convex code there can be the case where the operator has changed, causing no mint to happen.

Proof of Concept

The function Vault.claimRewards calculates the CVX reward that is expected, which is then meant to be transferred to AMPHClaimer and the minter:

...
 _totalCvxReward += _calculateCVXReward(_crvReward);
...
// Claim AMPH tokens depending on how much CRV and CVX was claimed
_amphClaimer.claimAmph(this.id(), _totalCvxReward, _totalCrvReward, _msgSender());
...
if (_totalCvxReward > 0) CVX.transfer(_msgSender(), _totalCvxReward);

The Cvx contract which performs the mint contains the following logic:

function mint(address _to, uint256 _amount) external {
    if(msg.sender != operator){
        //dont error just return. if a shutdown happens, rewards on old system
        //can still be claimed, just wont mint cvx
        return;
    }

The operator is the Booster contract which is called by BaseRewardPool which again is called by Vault.claimRewards. As can be seen in the snippet, the Convex protocol enables a shutdown case that wont break the BaseRewardPool, however the Vault implementation has not taken that case into consideration.

Severity deemed to be medium, due to high impact but special requirement.

Tools Used

Manual Review

Recommended Mitigation Steps

Check the CVX balance of the vault before and after the claim to assert that the correct CVX amount has been minted.

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xShaito marked the issue as sponsor confirmed

[0xShaito]

Impact: Users wouldn't be able to claim rewards anymore and accumulated rewards are lost. No user deposits at risk!

[c4-judge]

dmvt marked the issue as selected for report