search

code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

getMostPremium() does not necessarily return the best asset to trade for. #156

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

tensors

Vulnerability details

Impact

getMostPremium() often does not return the best asset after considering other fees. For example, since we are swapping weth to a stablecoin pool when we call it in line 120, we want to account for slippage, fees and price impact in the pool that _swap calls (a uniswap pool). These factors are often a lot larger than the small premiums given by curve pools.

Proof of Concept

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/strategies/NativeStrategyCurve3Crv.sol#L119-L120

and

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/strategies/NativeStrategyCurve3Crv.sol#L83

Recommended Mitigation Steps

In general it seems that getMostPremium() is used to decide which stablecoin a user should swap his funds to. The trading fees and slippage involved in this question are too complicated to be solved by just querying the curve pool for the smallest balance. Probably best to just let users decide how to get DAI, USDC, USDT.

GalloDaSballo commented 2 years ago

Agree with the finding, the check for curve liquidity indicates that the developers where trying to get the token that will yield the best swap However, uniswap will most likely be the swap path with the highest price impact

Not getting a quote from uniswap ends up being an oversight worth mitigating if the developer is trying to get getMostPemium