rvierdiiev - LibTWAPOracle may use outdated price

[sherlock-admin2]

rvierdiiev

medium

LibTWAPOracle may use outdated price

Summary

LibTWAPOracle may use outdated price, because when update it uses last balance stored in the curve twap, which may not be same as current timestamp.

Vulnerability Detail

LibTWAPOracle library is responsible for querying curve pool twap and based on the result calculate the price of UbiquityDollar.

Each time, when update is called, which can be called by anyone from facet for example, then in case if last prices update was before the last twap update, function will calculate prices, based on twap info. Also each time, it stores current data to use it later for comparison.

Now, let's check how twap oracle is implemented in the curve pool.
So each time, when _update function is called, then price_cumulative_last is updated with previous values. _update function is called only by other functions that somehow change liquidity amount or execute swaps. What this means is that in case if someone changes the price, then it will not be stored into the price_cumulative_last, until someone next will call _update. This can be quick or it can be long time without calling.

So, in case if someone calls TWAPOracleDollar3poolFacet.update, then it's possible that long time have passed since last update and that get_price_cumulative_last function will return not up to date values and thus average price will be incorrect.

Impact

Fetched twap price can be incorrect

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

The problem is that curve pool doesn't have ability to call _update function without a swap as it is possible in uniswap v2 twap. In this case ubiquity can implement it in their own contract and in case if block_timestamp_last from curve pool is not equal to block.timestamp, then increase get_price_cumulative_last(simulate _update function). But sometimes it can be not needed in case of small delays, so some threshold is needed to trigger update.

Duplicate of #13

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

The issue describes about TWAP can be manipulated because update function can be called anytime and by anyone, thus TWAP period can be as short as 1 block. It seems like a valid issue but after caeful consideration, it's noticed that the TWAP issue does not come from its period but the logic itself is incorrect, thus marking this as Invalid

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

The issue describes about TWAP can be manipulated because update function can be called anytime and by anyone, thus TWAP period can be as short as 1 block. It seems like a valid issue but after caeful consideration, it's noticed that the TWAP issue does not come from its period but the logic itself is incorrect, thus marking this as Invalid