c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Bug Description

The ethPerCvx() function relies on a Chainlink oracle to fetch the CVX / ETH price:

        try chainlinkCvxEthFeed.latestRoundData() returns (
            uint80 roundId,
            int256 answer,
            uint256 /* startedAt */,
            uint256 updatedAt,
            uint80 /* answeredInRound */
        ) {
            cl.success = true;
            cl.roundId = roundId;
            cl.answer = answer;
            cl.updatedAt = updatedAt;
        } catch {

The return values from latestRoundData() are validated as such:

VotiumStrategyCore.sol#L173-L181

        if (
            (!_validate ||
                (cl.success == true &&
                    cl.roundId != 0 &&
                    cl.answer >= 0 &&
                    cl.updatedAt != 0 &&
                    cl.updatedAt <= block.timestamp &&
                    block.timestamp - cl.updatedAt <= 25 hours))
        ) {

As seen from above, there is no check to ensure that cl.answer does not go below or above a certain price.

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. Therefore, if CVX experiences a huge drop/rise in value, the CVX / ETH price feed will continue to return minAnswer/maxAnswer instead of the actual price of CVX.

Currently, minAnswer is set to 1e13 and maxAnswer is set to 1e18. This can be checked by looking at the AccessControlledOffchainAggregator contract for the CVX / ETH price feed. Therefore, if CVX ever experiences a flash crash and its price drops to below 1e13 (eg. 100), the cl.answer will still be 1e13.

This becomes problematic as ethPerCvx() is used to determine the price of vAfEth:

VotiumStrategy.sol#L31-L33

    function price() external view override returns (uint256) {
        return (cvxPerVotium() * ethPerCvx(false)) / 1e18;
    }

Furthermore, vAfEth's price is used to calculate the amount of AfEth to mint to users whenever they call deposit():

AfEth.sol#L162-L166

        totalValue +=
            (sMinted * ISafEth(SAF_ETH_ADDRESS).approxPrice(true)) +
            (vMinted * vStrategy.price());
        if (totalValue == 0) revert FailedToDeposit();
        uint256 amountToMint = totalValue / priceBeforeDeposit;

If CVX experiences a flash crash, vStrategy.price() will be 1e13, which is much larger than the actual price of CVX. This will cause totalValue to become extremely large, which in turn causes amountToMint to be extremely large as well. Therefore, the caller will receive a huge amount of afEth.

Impact

Due to Chainlink's in-built circuit breaker mechanism, if CVX experiences a flash crash, ethPerCvx() will return a price higher than the actual price of CVX. Should this occur, an attacker can call deposit() to receive a huge amount of afEth as it uses an incorrect CVX price.

This would lead to a loss of funds for previous depositors, as the attacker would hold a majority of afEth's total supply and can withdraw most of the protocol's TVL.

Proof of Concept

Assume the following:

For convenience, assume that 1 safEth is worth 1 ETH.
The AfEth contract has the following state:
- ratio = 5e17 (50%)
- totalSupply() = 100e18
- safEthBalanceMinusPending() = 50e18
- vEthStrategy.balanceOf(address(this)) (vAfEth balance) is 50e18
The VotiumStrategy contract has the following state:
- Only 50 vAfEth has been minted so far (totalSupply() = 50e18).
- The contract only has 50 CVX in total (cvxInSystem() = 50e18).
- This means that cvxPerVotium() returns 1e18 as:
```
((totalCvx - cvxUnlockObligations) * 1e18) / supply = ((50e18 - 0) * 1e18) / 50e18 = 1e18
```

The price of CVX flash crashes from 2e15 / 1e18 ETH per CVX to 100 / 1e18 ETH per CVX. Now, if an attacker calls deposit() with 10 ETH:

priceBeforeDeposit, which is equal to price(), is 5e17 + 5e12 as:

safEthValueInEth = (1e18 * 50e18) / 1e18 = 50e18
vEthValueInEth = (1e13 * 50e18) / 1e18 = 5e14
((vEthValueInEth + safEthValueInEth) * 1e18) / totalSupply() = ((50e18 + 5e14) * 1e18) / 100e18 = 5e17 + 5e12

Since ratio is 50%, 5 ETH is staked into safEth:
- sMinted = 5e18, since the price of safEth and ETH is equal.
The other 5 ETH is deposited into VotiumStrategy:
- priceBefore, which is equal to cvxPerVotium(), is 1e18 as shown above.
- Since 1 ETH is worth 1e16 CVX (according to the price above), cvxAmount = 5e34.
- Therefore, vMinted = 5e34 as:
```
mintAmount = ((cvxAmount * 1e18) / priceBefore) = ((5e34 * 1e18) / 1e18) = 5e34 
```

To calculate vStrategy.price() after VotiumStrategy's deposit() function is called:

ethPerCvx() returns 1e13, which is minAnswer for the CVX / ETH price fee.

cvxPerVotium() is still 1e18 as:

supply = totalSupply() = 5e34 + 50e18
totalCvx = cvxInSystem() = 5e34 + 50e18
((totalCvx - cvxUnlockObligations) * 1e18) / supply = ((5e34 + 50e18 - 0) * 1e18) / (5e34 + 50e18) = 1e18

Therefore vStrategy.price() returns 1e13 as:

(cvxPerVotium() * ethPerCvx(false)) / 1e18 = (1e18 * 1e13) / 1e18 = 1e13

To calculate the amount of AfEth minted to the caller:

totalValue = (5e18 * 1e18) + (5e34 * 1e13) = 5e47 + 5e36
amountToMint = totalValue / priceBeforeDeposit = (5e47 + 5e36) / (5e17 + 5e12) = ~1e30

As seen from above, the attacker will receive 1e30 AfEth, which is huge compared to the remaining 100e18 held by previous depositors before the flash crash.

Therefore, almost all of the protocol's TVL now belongs to the attacker as he holds most of AfEth's total supply. This results in a loss of funds for all previous depositors.

Recommended Mitigation

Consider validating that the price returned by Chainlink's price feed does not go below/above a minimum/maximum price:

VotiumStrategyCore.sol#L173-L181

        if (
            (!_validate ||
                (cl.success == true &&
                    cl.roundId != 0 &&
-                   cl.answer >= 0 &&
+                   cl.answer >= MIN_PRICE &&
+                   cl.answer <= MAX_PRICE &&
                    cl.updatedAt != 0 &&
                    cl.updatedAt <= block.timestamp &&
                    block.timestamp - cl.updatedAt <= 25 hours))
        ) {

This ensures that an incorrect price will never be used should CVX experience a flash crash, thereby protecting the assets of existing depositors.