sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

fugazzi - TWAP oracle returns incorrect price #192

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

fugazzi

high

TWAP oracle returns incorrect price

Summary

The TWAP oracle is using a Curve metapool and incorrectly fetches the price using the LP token of the base pool instead of the underlying tokens.

Vulnerability Detail

Curve metapools are a type of pool that pairs a token, uAD in this case, with another Curve pool, here 3CRV. The intention is to take advantage of an existing pool liquidity when deploying a new pool, which means that the tokens in the metapool are the main token (uAD) and the LP token (3CRV).

The implementation of the TWAP oracle is using the get_dy() function, which calculate the output (dy) of a swap, but since this is a metapool this amount will be expressed in terms of the LP token, which is not the price of the underlying tokens in the base pool (remember the LP token has the accumulated fees).

This means that the uAD oracle will return the price in terms of the LP token, which will be significantly lower than the real correct price.

We can clearly check the issue using the deployed version of TWAPOracle (0x7944d5b8f9668AfB1e648a61e54DEa8DE734c1d1) that contains the same problem. The price of uAD returned by the oracle is 0.9334:

❯ cast call --rpc-url https://eth.llamarpc.com 0x7944d5b8f9668AfB1e648a61e54DEa8DE734c1d1 "price0Average()(uint256)"
933492204132480181 [9.334e17]

However the current price of uAD is 0.9570 (checked using Curve UI at https://curve.fi/#/ethereum/pools/factory-v2-330/swap):

swap

Impact

The oracle price for uAD is incorrect. This will impact the calculations in getDollarPriceUsd() which is used in mintDollar() and redeemDollar().

Other contracts not in scope are also likely impacted (LibChef, LibCreditNftManager, LibCurveDollarIncentive).

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/deprecated/TWAPOracle.sol#L57-L59

Tool used

Manual Review

Recommendation

The oracle should use get_dy_underlying() which returns the correct price expressed in terms of the underlying tokens of the base pool.

            // price to exchange amounIn uAD to 3CRV based on TWAP
-           price0Average = IMetaPool(pool).get_dy(0, 1, 1 ether, twapBalances);
+           price0Average = IMetaPool(pool).get_dy_underlying(0, 1, 1 ether, twapBalances);
            // price to exchange amounIn 3CRV to uAD  based on TWAP
-           price1Average = IMetaPool(pool).get_dy(1, 0, 1 ether, twapBalances);
+           price0Average = IMetaPool(pool).get_dy_underlying(0, 1, 1 ether, twapBalances);

Duplicate of #59

sherlock-admin2 commented 6 months ago

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

auditsea commented:

REF #030

sherlock-admin2 commented 6 months ago

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

auditsea commented:

REF #030