code-423n4 / 2021-11-vader-findings

0 stars 0 forks source link

Unbounded loop in TwapOracle.update can result in oracle being locked #8

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

TomFrench

Vulnerability details

Impact

Loss of ability of TwapOracle to update should too many pools be added.

Proof of Concept

TwapOracle allows an unlimited number of pairs to be added and has no way of removing pairs after the fact. At the same time TwapOracle.update iterates through all pairs in order to update value for each pair.

https://github.com/code-423n4/2021-11-vader/blob/3a43059e33d549f03b021d6b417b7eeba66cf62e/contracts/twap/TwapOracle.sol#L322-L369

TwapOracle.registerPair is a permissioned function so that only the owner can add new pairs however should the owner account be compromised or not mindful of the number of pairs being added it is possible to put the oracle into a state in which it is unable to update. The oracle cannot recover from this state.

Recommended Mitigation Steps

Possible options:

SamSteinGG commented 2 years ago

The severity of the finding should be reduced as it relies on ill behavior from its owner which is a multisignature address.

alcueca commented 2 years ago

The severity rating is valid since the signers might not be aware of the limitation, or the limitation might be reached by natural means.

SamSteinGG commented 2 years ago

The TWAP oracle module has been completely removed and redesigned from scratch as LBTwap that is subject of the new audit.