The TWAP interval is too short which makes manipulating the price easier

Summary

The interval to get the TWAP of the Curve metapool is extremely short which makes oracle manipulation much easier and the price of uAD can be manipulated.

Vulnerability Detail

The TWAP oracle uses the Curve Metapool to get the price of uAD, lets take a look at the update() function:

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibTWAPOracle.sol#L74

function update() internal {
        TWAPOracleStorage storage ts = twapOracleStorage();
        (
            uint256[2] memory priceCumulative,
            uint256 blockTimestamp
        ) = currentCumulativePrices();
        if (blockTimestamp - ts.pricesBlockTimestampLast > 0) {
           //price is updated
        }

As you can see here 1 block price is used as the TWAP window which makes the oracle a spot oracle and manipulation is much easier because proposing 2 blocks consecutively would be enough for the attacker to manipulate the price.

Impact

The attacker can abuse this to manipulate the price as he wants. The TWAP is used in many places like for example LibCurveDollarIncentive.sol so the attacker can manipulate the price and profit from this.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibTWAPOracle.sol#L74

Tool used

Manual Review

Recommendation

The TWAP Oracle should have a bigger interval because the manipulation cost increases with the size of the TWAP window. It should also be flexible to be readjusted according to market conditions.

if (blockTimestamp - ts.pricesBlockTimestampLast > interval) {
      //price is updated
}

Duplicate of #20

sherlock-admin2 commented 7 months ago

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