CvgCvxStakingPositionService.sol#_convertCvxToCvgCvx() - slippage is not checked for opposite direction depeg

[hats-bug-reporter[bot]]

Github username: @PlamenTSV Twitter username: @p_tsanev Submission hash (on-chain): 0xd86efa8167334495b62158f1fe337192df37e27f4d5e62ec825e4aac3cffb259 Severity: medium

Description: Description\ The _convertCvxToCvgCvx() is meant to be executed when a user deposits ETH directly, like so:

1. ETH is deposited, converted to WETH and exchanged for CVX
2. Calculate fees amount and check slippage by viewing the current Curve exchange rate and comparing it to the (amount-fees) + depeg percentage. A problem can occur at this step since the check is only done with >, disregarding the possibility of a depeg to the lower direction.

Attack Scenario\ If we plug in numbers: cvxAmount = 1000, fees = 2%, thus the real amount would be 980. Inside the if check:

if (_curvePool.get_dy(0, 1, cvxAmount) > ((cvxAmount - feesAmount) * depegPercentage) / HUNDRED)

Per the current 2.5% depeg percentage, the condition becomes:

current exchRate > 980*1025/1000 = 1004.5 (1004 for solidity)

If the condition holds true we:

1. Check the slippage require(minCvgCvxAmountOut >= cvxAmount)
2. Swap CVX to cvgCVX due to the depeg to make sure the user is not underpaid

If the condition is false we directly mint the cvgCVX assuming 1:1 ratio. However the code never consideres the scenario where a depeg can occur in the opposite direction, where the exchange rate instead of giving us more cvgCVX per CVX, gives us less. In that case the code would still mint cvgCVX to the user in a 1:1 ratio, technically minting more token. With a ratio of cvgCVX:CVX = 1.03:1, we would swap 1000 cvx for 1030 cvgCVX, triggering the depeg condition and swapping. But an opposite depeg with the same rate would mean 1000 CVX gets us 970 cvgCVX. (cvgCVX:CVX = 1:1.03) The code does not cover this an would instead mint 1:1, granting us a 30 cvgCVX bonus to our position

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

   if (_curvePool.get_dy(0, 1, cvxAmount) > ((cvxAmount - feesAmount) * depegPercentage) / HUNDRED) {
   require(minCvgCvxAmountOut >= cvxAmount, "INVALID_SLIPPAGE");
   
   /// @dev peg is too low, we swap in the LP with the CVX sent
   cvx1.mint(address(this), cvxAmount);
   _curvePool.exchange(0, 1, cvxAmount, minCvgCvxAmountOut, address(this));
   + } else if(_curvePool.get_dy(0, 1, cvxAmount) < ((cvxAmount - feesAmount) * HUNDRED) / depegPercentage){
   + require(minCvgCvxAmountOut <= cvxAmount, "INVALID_SLIPPAGE");
   +    
   +    cvx1.mint(address(this), cvxAmount);
   +    _curvePool.exchange(0, 1, cvxAmount, minCvgCvxAmountOut, address(this));
   +} 
   else {
   /// @dev peg OK, we pass through the mint process 1:1
   cvxConvergenceLocker.mint(address(this), cvxAmount, false);
   }

Recommendation\ Just like for the greater depeg, make sure to check for the lower depeg as well, to make sure the user is not getting overminted tokens. You could use the same depeg percentage if it satisfies the protocol's assumptions, the above scenario has a 3% depeg, which is higher than the current configuration.

[shalbe-cvg]

Hello,

Thank you for reporting this issue. What you have mentioned is the expected behavior of this feature, in the case where the peg is unfavorable to the user, we directly mint the cvgCVX tokens so the user is assured of having a 1:1 ratio.

Regards.