Handle

leastwood

Vulnerability details

Impact

The TwapOracle.consult() function iterates over all token pairs which belong to either VADER or USDV` and then calculates the price of the respective asset by using both UniswapV2 and Chainlink price data. This helps to further protect against price manipulation attacks as the price is averaged out over the various registered token pairs.

If a new asset is added by first registering the token pair and aggregator, the consultation result for that token pair will remain skewed until the next update interval. This is due to the fact that the native asset amount will return 0 due to the default price1Average value being used. However, the Chainlink oracle will return a valid result. As a result, the query will be skewed in favour of sumUSD resulting in incorrect consultations.

I'd classify this issue as high risk as the oracle returns false results upon being consulted. This can lead to issues in other areas of the protocol that use this data in performing sensitive actions.

Proof of Concept

https://github.com/code-423n4/2021-11-vader/blob/main/contracts/twap/TwapOracle.sol#L115-L157 https://github.com/code-423n4/2021-11-vader/blob/main/contracts/twap/TwapOracle.sol#L314 https://github.com/code-423n4/2021-11-vader/blob/main/contracts/twap/TwapOracle.sol#L322-L369

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider performing proper checks to ensure that if pairData.price1Average._x == 0, then the Chainlink aggregator is not queried and not added to sumUSD. Additionally, it may be useful to fix the current check to assert that the pairData.price1Average.mul(1).decode144() result is not 0, found here. require(sumNative != 0) is used to assert this, however, this should be require(pairData.price1Average.mul(1).decode144() != 0) instead.

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