code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

Swapper3Crv's swapping path can be suboptimal #184

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/swappers/Swapper3Crv.sol#L193-L195

Vulnerability details

Impact

Swapper3Crv.swap() result can be suboptimal as only paths with ETH are evaluated.

Setting severity to medium as despite function availability not affected there can be some fund losses as a result.

Proof of Concept

_tokenAmountOut uses fixed [tokenIn, ETH, tokenOut] path to estimate the output:

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/swappers/Swapper3Crv.sol#L193-L195

However, for some token combinations direct [tokenIn, tokenOut] swap can provide better results:

https://info.uniswap.org/#/pools

Particularly given that tokenIn is DAI / USDC / USDT:

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/swappers/Swapper3Crv.sol#L125-L127

Recommended Mitigation Steps

Add direct DAI-USDC, DAI-USDT, USDC-USDT paths as the corresponding pools are liquid and can provide better execution.

chase-manning commented 2 years ago

Contract is out of scope of the audit