code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

A token quickly losing value will block liquidations #201

Closed c4-bot-5 closed 5 months ago

c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseOracleHub/OracleHelper.sol#L131-L174

Vulnerability details

Impact

The protocol uses the WiseOracleHub contract to compute the value of each token by calling the getTokensInETH function. Inside there, the high overview is that it takes the Chainlink price feed for the token in terms of ETH and a TWAP computation of the price of the asset. Having these 2 results, the difference is calculated and if it is greater than 2.5% the call is reverted.

The TWAP mechanism is used in order to mitigate the price manipulation of AMMs by computing the price over a certain amount of time. This feature is also risky when a quick increase or decrease of value happens because since it returns the average price over time, it will need some time to reach the new price. That means that for a certain interval of time, the value returned by the TWAP will be outdated. As happened recently with the Uniswap token, it increased really quickly in price due to enabling the fees into the protocol. That lead to protocols like Compound to borrow this asset to a really low price compared to the actual one due to the TWAP result.

In the particular case of WiseLending, since it checks for the difference of the result of Chainlink price feed and TWAP, it disable this scenario of a huge increase in a short amount of time because when someone would try to borrow from this asset, the function would revert due to the price difference. However, for a huge decrease of value like a depeg it can lead to a really bad situation for the protocol and for users.

  1. During a huge decrease of price of an asset, the liquidation for positions that borrowed this asset will revert due to this discrepancy of price. That could lead to bad debt for the protocol because nobody would be able to liquidate the position.

  2. During a huge decrease of price of an asset, withdrawals of this same asset would be disabled for positions that have some amount borrowed of any asset. That is because this function will check for the healthState by calculating the value of this asset and reverting. If the user wants to withdraw any of his collateral assets, he will be forced to repay all his borrowed amounts before being able to withdraw any asset. This can be really bad for the user because he could get this asset back and swap it to stablecoins for example before losing too much value.

Tools Used

Manual review

Recommended Mitigation Steps

I would recommend to revert when the difference of price exceeds 2.5% if the chainlink price is greater than the TWAP, but when the chainlink price would be lower than TWAP use the chainlink price to disable this situation where positions can not be liquidated.

Notice that the chainlink price is checked to be in a certain range, so if it does not return a reasonable value it will revert anyways.

Assessed type

Oracle

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as duplicate of #91

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as sufficient quality report

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Out of scope