Missing slippage control while depositing rewards in SafEth and VotiumStrategy

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L272-L293

Vulnerability details

Summary

Deposits to SafEth and VotiumStrategy coming from rewards lack slippage control, making them susceptible to sandwich attacks by MEV bots, which can result in a loss of funds for the protocol.

Impact

Rewards coming from the VotiumStrategy contract are compounded and deposited back to the protocol using the depositRewards() function:

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L272-L293

272:     function depositRewards(uint256 _amount) public payable {
273:         IVotiumStrategy votiumStrategy = IVotiumStrategy(vEthAddress);
274:         uint256 feeAmount = (_amount * protocolFee) / 1e18;
275:         if (feeAmount > 0) {
276:             // solhint-disable-next-line
277:             (bool sent, ) = feeAddress.call{value: feeAmount}("");
278:             if (!sent) revert FailedToSend();
279:         }
280:         uint256 amount = _amount - feeAmount;
281:         uint256 safEthTvl = (ISafEth(SAF_ETH_ADDRESS).approxPrice(true) *
282:             safEthBalanceMinusPending()) / 1e18;
283:         uint256 votiumTvl = ((votiumStrategy.cvxPerVotium() *
284:             votiumStrategy.ethPerCvx(true)) *
285:             IERC20(vEthAddress).balanceOf(address(this))) / 1e36;
286:         uint256 totalTvl = (safEthTvl + votiumTvl);
287:         uint256 safEthRatio = (safEthTvl * 1e18) / totalTvl;
288:         if (safEthRatio < ratio) {
289:             ISafEth(SAF_ETH_ADDRESS).stake{value: amount}(0);
290:         } else {
291:             votiumStrategy.depositRewards{value: amount}(amount);
292:         }
293:     }

As we can see in lines 289 and 291, depending on the current TVL ratio between both collaterals, rewards will be either staked in SafEth or deposited back to VotiumStrategy.

Both scenarios experience the same lack of slippage control. The call to stake() sets zero as the _minOut argument in SafEth. Similarly, in the depositRewards() function of VotiumStrategy, the process involves using a Curve Pool to exchange ETH for CVX tokens with a hardcoded min_dy parameter set to zero.

In any case, the reward compounding transaction could be sandwiched by a MEV bot, causing less output received and a loss of funds to the protocol in general.

Recommendation

Add slippage control to the deposit rewards flow in order to allow the rewarded role to specify a minimum output of assets when rewards are either staked in SafEth, or swapped for CVX in VotiumStrategy.

Assessed type

MEV

[0xleastwood]

One part of this issue (slippage in depositRewards()) is a duplicate of #39 and the implementation of ISafEth(SAF_ETH_ADDRESS).stake() does not allow for any form of sandwich attack as far as I can see but will leave as primary for the sponsor to confirm. Keep in mind, there is no provided POC to showcase how this might be exploited in the case of the SAF_ETH_ADDRESS contract.

[c4-judge]

0xleastwood marked the issue as primary issue

[c4-judge]

0xleastwood marked the issue as satisfactory

[c4-judge]

0xleastwood marked the issue as selected for report

[c4-judge]

0xleastwood removed the grade

[elmutt]

One part of this issue (slippage in depositRewards()) is a duplicate of #39 and the implementation of ISafEth(SAF_ETH_ADDRESS).stake() does not allow for any form of sandwich attack as far as I can see but will leave as primary for the sponsor to confirm. Keep in mind, there is no provided POC to showcase how this might be exploited in the case of the SAF_ETH_ADDRESS contract.

I believe our solution for #39 (https://github.com/asymmetryfinance/afeth/pull/169) should solve this but please let us know if you see any way this can still be exploited

[c4-sponsor]

elmutt (sponsor) confirmed

[0xleastwood]

Technically this will fix the issue, but I guess I wanted to confirm if it is possible to sandwich attack the ISafEth(SAF_ETH_ADDRESS).stake{value: amount}(0) call in the first place? @elmutt

[elmutt]

Technically this will fix the issue, but I guess I wanted to confirm if it is possible to sandwich attack the ISafEth(SAF_ETH_ADDRESS).stake{value: amount}(0) call in the first place? @elmutt

When you call stake() on safEth its buying 6 different LSD derivatives (reth, lido, ankr, swell, stafi, ankr) and will revert if the price paid for any of them is 1% or higher from the oracle price. You can see the revert in finalChecks() in derivativeBase.sol here: https://etherscan.io/address/0xb3e64c481f0fc82344a7045592284fddb9905b8b#code

Im not really an expert on sandwich attacks, would this mean it could be sandwiched for up to 1% if no minout is passed?

[0xleastwood]

Right, I see. You need some tolerance for slippage to allow for a trade to execute in the first place. I think this function is being restricted anyway, so no way to sandwich attack. But it seems possible to extract value still.

[c4-judge]

0xleastwood marked the issue as duplicate of #23

[c4-judge]

0xleastwood changed the severity to 3 (High Risk)

[c4-judge]

0xleastwood marked the issue as not selected for report

[c4-judge]

0xleastwood marked the issue as satisfactory