sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

mstpr-brainbot - Univ3 oracle can be manipulated if pool is illiquid or non-existed #107

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

mstpr-brainbot

high

Univ3 oracle can be manipulated if pool is illiquid or non-existed

Summary

Some tokens can be stolen from the swapper contract via oracle manipulation.

Vulnerability Detail

In the Diversifier system, a PassThroughWallet receives ERC20 tokens and sends them to Split for distribution. Split then distributes the tokens to recipients, who are likely swappers. Swappers receive any token and are obligated to swap it for its beneficiary token, provided a Uniswap v3 pool exists for the sellToken - beneficiaryToken pair. However, this design has a vulnerability due to the permissionless nature of Uniswap v3 pools.

Consider a swapper whose responsibility is to swap any received tokens for DAI. If Split receives LUSD, which doesn't have a specified Uniswap pool, the swapper will use the default 0.3% fee tier pool for LUSD-DAI. However, stable-stable pools like LUSD-DAI generally use a lower 0.05% fee tier, and the 0.3% tier pool is likely to be illiquid.

An attacker can exploit this illiquidity by adding a small amount of LUSD and an even smaller amount of DAI to the pool, effectively lowering the LUSD price relative to DAI (e.g., 1 LUSD == 0.0001 DAI). When the Uniswap v3 oracle updates, the attacker can exchange a large amount of LUSD for a small amount of DAI from the swapper, making a significant profit.

Similarly, if the pool didn't exist, the attacker could create one with distorted token amounts (e.g., 1 LUSD and 1 Wei worth of DAI), causing the DAI price to be extremely high relative to LUSD. The attacker would then receive all the LUSD in the swapper contract while returning a minuscule amount of DAI.

Impact

Since Split and PassThroughWallet can receive any tokens, and the swappers' goal is to swap any ERC20 to its beneficiary token, this vulnerability is likely to occur. The swapper owner needs to anticipate the possibility of receiving tokens like LUSD and make the appropriate adjustments. However, since funding the PassThroughWallet and Split is permissionless, the risk is already present for the swapper once the contract receives LUSD.

In cases where pool liquidity is extremely low, like the LUSD-DAI (0.3%) example with only $20 TVL, the pool can be easily manipulated. If the pool does not exist, an attacker can create one with a fake price by adding a minimal amount of liquidity (e.g., 1 LUSD and 1 Wei worth of DAI). In summary, both low liquidity pools and non-existent pools pose threats to the swapper. Since these issues stem from the same problem, they can be addressed together in a single security finding.

Code Snippet

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-pass-through-wallet/src/PassThroughWalletImpl.sol#L120-L136 Once the funds received in PassThrough anyone can send it to PassThrough's beneficiary which is most likely split.

As stated in Splits docs, funds in Splits can be sent to recipients (swappers in diversifier case) permissionless. Anyone can do it.

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L231

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-oracle/src/UniV3OracleImpl.sol#L248-L285

Only checks if the pool exists or not, not checks the liquidity it holds.

Tool used

Manual Review

Recommendation

Check the liquidity on pool. Maybe add an another variable to PairOverride struct as defaultLiquidityThreshold and check if the default liquidity threshold satisfied on the pool. If not, revert because the price quoting on low liquidity pools will not be healthy/accurate.

Duplicate of #26