TWAP interval is hardcoded as 30 minutes

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L87

Vulnerability details

Impact

TWAP oracles are designed to be more resistant to price manipulation,but they are not immune . Hardcoding twap interval or making it immutable makes it easy to manipulate. Although the check for minimum liquidity pool amount makes it harder for a attacker to manipulate it , but there is still a chance to manipulate it since the time interval is just 30 mins there is still a slight chance that the price can be manipulated by some amount.So the attacker will need to use their capital to inflate the price a flashloan wouldn't work here.[since it is a time-weighted oracle, so time(blocks) must first pass before the desired price is incorporated]

Let's say a asset has price of $100.

A 5% decrease in the market price of the asset ($100) means the price needs to move to $95.

For each block, to achieve this 5% movement, traders would execute transactions that push the price towards $95.

The cost incurred in each block due to arbitrage and fees is constant. Let’s assume it's $500 per block.

Therefore, for 30 blocks (30 min), the total cost would be 30 blocks * $500/block = $15,000.

[NOTE:

although avg block speed is 13s , here i assumed just for simplicit calculation, since arbitraging can be done by flashbots, and nobody and can pick up, & it could go on for quite some time until someone notices]

Although it is a huge cost for a arbitrager but it still might be profitable for him through the callerfee liquidation . and also they can place arbitrary large orders at arbitrary large or small price points => do this continuosly to influence TWAP & use the manipulated twap for profit

Proof of Concept

Although the the protocol has done several checks for stopping price manipulation, but if the relation (chainlinkDiff <= twapDiff) is false the function returns the twap price (where also a further check is done for small liquidty pool, this additional check also makes it hard to manipulate the price)

 else {
                    // Check valid twap liquidity
                    IERC20 weth = IERC20(C.WETH);
                    uint256 wethBal = weth.balanceOf(C.USDC_WETH);
                    if (wethBal < 100 ether) {
                        return chainlinkPriceInEth;
                    }
                    return twapPriceInEth;

But let's just say an attacker (a shorter on the protocol) wants to liquidate some shorter, they can just inflate the prices maybe by some percent which makes the shorter undercollateralize and the attacker can liquidate him and recieve the caller fee. Another instance is that a bidder may also have to provide more collateral on his buying than the required amount .

 try IDiamond(payable(address(this))).estimateWETHInUSDC(C.UNISWAP_WETH_BASE_AMT, 30 minutes) returns (uint256 twapPrice)

Tools Used

Manual Review

Recommended Mitigation Steps

Make a variable called twapseconds in LibOracle.sol uint private twapseconds = 30 minutes and then change this line

- try IDiamond(payable(address(this))).estimateWETHInUSDC(C.UNISWAP_WETH_BASE_AMT, 30 minutes) returns (uint256 twapPrice)
+ try IDiamond(payable(address(this))).estimateWETHInUSDC(C.UNISWAP_WETH_BASE_AMT, twapseconds) returns (uint256 twapPrice)

Also add a function for changing the twap duration

[NOTE:

Usually the 30 min interval is ok , and since the check for huge liquidty pool is also done, there is not much of a problem, but if the chainlink price differs by some percent and twap also lags in price movements(due to hardcoded interval) then it is a big problem since after the chainlink failure this is the only source of truth]

[ NOTE:

The recent Compound V2 exploitation underscores the limited use of TWAP, not only because of their inherently incorrect price reflection but also by manipulating the price used by the TWAP oracle, attackers can create favourable conditions which can finally lead to sophisticated attacks ]

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

It's protocol's intended design with no actual threat entailed other than the one hypothetically described. QA at best.

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-c