sherlock-audit / 2023-05-USSD-judging

9 stars 7 forks source link

Bahurum - Incorrect rebalancing due to unrelated Uni V3 pool reserves #792

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bahurum

high

Incorrect rebalancing due to unrelated Uni V3 pool reserves

Summary

The assumption that reserves of the USSD/DAI pool will be bounded by the constant product formula does not hold true for Uniswap v3. This breaks the rebalancing mechanism.

Vulnerability Detail

USSDRebalancer assumes that in an UniV3 pool the dollar values of the 2 reserves are equal when the pool is balanced (market price). This is true in Uniswap v2 but not for v3. See the USD/DAI pool for example.

This means that the rebalancing amount is incorrect and causes several issues:

Impact

Rebalanicing mechanism won't work as expected, making the protocol unable to keep the peg and vulnerable to theft of funds.

Code Snippet

https://github.com/USSDofficial/ussd-contracts/blob/f44c726371f3152634bcf0a3e630802e39dec49c/contracts/USSDRebalancer.sol#L92-L107

Tool used

Manual Review

Recommendation

There is really no way in UniV3 to rebalance starting from reserve amounts. exactInputSingle() with sqrtPriceLimitX96 set to the value equivalent to 1:1 price can be used in SellUSSDBuyCollateral() to swap USSD for DAI. But this won't work for BuyUSSDSellCollateral() as the amounts of each collateral to swap for DAI cannot easily be known before-hand.

Duplicate of #808

bahurum commented 1 year ago

Escalate for 10 USDC.

This is a duplicate of #808.

sherlock-admin commented 1 year ago

Escalate for 10 USDC.

This is a duplicate of #808.

  • They describe the same underlying wrong assumption: From 808: However, due to the different designs of Uniswap v3 and Uniswap v2, the proportion of pool tokens does not necessarily correspond to the price Here: USSDRebalancer assumes that in an UniV3 pool the dollar values of the 2 reserves are equal when the pool is balanced (market price). This is true in Uniswap v2 but not for v3

  • They both describe underflow in USSDamount - DAIamount as a consequence

  • They describe the same attack vector: the difference is that here it is symmetrical respect to 808 (DAI and USSD are inverted in the attack, depeg vs overpeg, etc..)

  • Same impact: failing rebalance + theft of funds with price manipulation

  • The recommendation here says that cannot fix this only using reserves. 808 says that also liquidity ranges should be taken into account in order to fix this.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

ctf-sec commented 1 year ago

Valid duplicate of #808

hrishibhat commented 1 year ago

Result: High Duplicate of #808

sherlock-admin commented 1 year ago

Escalations have been resolved successfully!

Escalation status: