search

code-423n4 / 2022-09-canto-findings

0 stars 0 forks source link

Using an average of 8 periods, each period covering 30 minutes may cause the price to be extremely delayed. Hackers can profit from these delays. And may cause bad debt. #128

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L524-L593 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L45

Vulnerability details

Impact

Using an average of 8 periods, each period covering 30 minutes may cause the price to be extremely delayed. Hackers can profit from these delays.

In crypto, the price can +100% in just 30 minutes. Imagine volatility of 8 * 30 = 240 minutes = 4 hours. Hackers can profit a lot from these delays. Moreover, if the market crash just like UST, the liquidator can't liquidate assets in time due to extremely delayed price, causing bad debt.

Proof of Concept

uint public periodSize = 1800;

Every 30 minutes one observation will be stored.

...
uint price = IBaseV1Pair(pair).quote(address(token), decimals, 8);
...
        for(uint i; i < 8; ++i) {
            uint token0TVL = assetReserves[i] * (prices[i] / decimals);
            uint token1TVL = unitReserves[i]; // price of the unit asset is always 1
            LpPricesCumulative += (token0TVL + token1TVL) * 1e18 / supply[i];
        }
        uint LpPrice = LpPricesCumulative / 8; // take the average of the cumulative prices 
...

Many functions in BaseV1-periphery use an average of 8 periods = 240 minutes = 4 hours which is ridiculously long.

Recommended Mitigation Steps

Reduce periodSize to 5 minutes

nivasan1 commented 2 years ago

The Pricing in the lending market is meant to be as resistant to large swings in volatility as possible to protect suppliers in each cToken market, changing to a shorter period size leaves the possibility of attackers manipulating prices through flash-loans over time. Also the period size can be changed through governance in the case that the community decides to decrease the TWAP.

0xean commented 2 years ago

This is a design tradeoff on the "correct" time for the TWAP, hard to see it as a critical vulnerability. Downgrading to QA. Warden has no QA report, so this will stand as theirs.