search

code-423n4 / 2022-05-bunker-findings

1 stars 0 forks source link

`UniswapV2PriceOracle.sol` Requires Constant Upkeep Otherwise It May Lead To Liquidation #111

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L23-L48 https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L126-L144 https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L170-L178

Vulnerability details

Impact

The Uniswap oracle utilises a 30 minute TWAP window which helps to protect the protocol from flash loan manipulation. However, in order to keep this data fresh, the update() function must be called by some individual on each 30 minute interval. Failure to do so will result in an extended TWAP window which may return a drastically different price when compared to the 30 minute TWAP window. As a result, if the Uniswap oracle is not updated for an extended period of time and a number of individuals borrow based on this information, a subsequent update may lead to a liquidation as the 30 minute TWAP price is lower than the currently referenced price.

The sponsor has stated that they will be in charge of constantly upkeeping this oracle, however, this is a centralised point of failure and there is no guarantee or incentive that this will continue in the long run. I strongly believe that protocols need to be self-sufficient and they should not be reliant on some benevolent actor.

Recommended Mitigation Steps

Consider integrating some sort of incentive behaviour into the Uniswap oracle model to ensure price information is updated on each update interval. There are a couple of implementations which verify block data on-chain, ensuring past price information is valid, however, this has serious overhead gas costs due to on-chain verification. Therefore, I believe some sort of on-chain incentive mechanism is ideal in this situation.

bunkerfinance-dev commented 2 years ago

We are aware of this and will eat the costs of upkeeping the oracle for now.

gzeoneth commented 2 years ago

Downgrading to Low / QA.

gzeoneth commented 2 years ago

Treating as warden's QA report.