c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategyCore.sol#L302-L304

Vulnerability details

Vulnerability Details

Upon claiming Votium rewards, applyRewards() is intended to be invoked bi-weekly in order to exchange the tokens for eth and put the eth received back into the strategies. Based on the current ratio it either stakes the amount into safETH or obtains some CVX by selling eth on Curve and then locks them to get vlCVX.
- ```
uint256 safEthRatio = (safEthTvl * 1e18) / totalTvl;
if (safEthRatio < ratio) {
ISafEth(SAF_ETH_ADDRESS).stake{value: amount}(0);
} else {
votiumStrategy.depositRewards{value: amount}(amount);
}
```
Let's say the safEthRatio < ratio, which triggers ISafEth(SAF_ETH_ADDRESS).stake{value: amount} being invoked. And if the amount < ISafEth(SAF_ETH_ADDRESS).minAmount. The whole re-investing strategy collapses.
As of Sep. 2023, the Votium rewards are 0,000016 eth for 1 vlCVX per round, it means, we need at least 3125 vlCVX being delegated to the Votium in order to pass the threshold.

Impact

This could backfire in early stages or during certain market conditions, where the amount of vlCVX hold by afETH is not enough to generate 0.05 eth bi-weekly. Basically, that forces to flows that are inconsistent with the main re-investment flow proposed by Asymmetry, which ultimately could result in theft as demonstrated in the issue #15.

Recommended Mitigation Steps

Short term:
- Wrapp IAfEth(manager).depositRewards into the try/catch block. And if one of the following conditions arises:
- The min safETH deposit amount is not reached
- Chainlink price feed could not be validated
- Low-level call which sends fees fails
just simply invoke VotiumStrategy.depositRewards().
```
  if (address(manager) == address(0)) {
      depositRewards(ethReceived);
  } else {
      try IAfEth(manager).depositRewards{value: ethReceived}(ethReceived) {}
      catch {depositRewards(ethReceived);}
  }
```
Long term: N/A

Assessed type

Context

elmutt commented 10 months ago

Nice find. At first glance it doesnt seem to matter but when you pointed out out early market conditions resulting in less rewards it makes total sense that it will be a problem we will likely encounter

Rassska commented 9 months ago

Ooppss, the reward threshold defined here should be changed to:

$$ \frac{0,000016} {ratio} * \sum_{i = 0}^{lockedBalances.length - 1} lockedBalances[i] $$

Meaning that:

if the ratio = 1e18 => the rewards > 0,05eth
if the ratio = 5e17 => the rewards > 0,1eth
....

elmutt commented 9 months ago

Thanks! After discussing internally we decided to solve this by calling setMinAmount(0) on the safEth contract.

Will update this issue when thats done

c4-judge commented 9 months ago

0xleastwood marked the issue as primary issue

c4-judge commented 9 months ago

0xleastwood marked the issue as selected for report

c4-sponsor commented 9 months ago

elmutt (sponsor) confirmed