sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

5 stars 5 forks source link

den_sosnovskyi - `VeloSwapUtils` contract uses different route types for same `IVeloRouter.execute` function for Uniswap V2 router #103

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

den_sosnovskyi

medium

VeloSwapUtils contract uses different route types for same IVeloRouter.execute function for Uniswap V2 router

Summary

VeloSwapUtils contract has several swap functions, which call Velo Router execute function for uniswap v2 router with different route argument

Vulnerability Detail

VeloSwapUtils contract first swap function can call Velo Router execute function with V2_SWAP_EXACT_IN type - so it calls Uniswap V2 router under the hood. For this they pass inputs argument. The inputs argument encodes route argument, which is an array of addresses. The same logic happens in the second swap function, they call same contract for uniswap v2, but for the route argument they use not array of addresses, but an array of IVeloRouter.Route struct, which is correct. If we see the code of the Velo Router they call, it calls dispatch function, which parse route for Uniswap v2 as IVeloRouter.Route struct. Further, they try to read the struct data in uniswap router. So the first swap function in the VeloSwapUtils contract should also pass IVeloRouter.Route struct

Impact

The first VeloSwapUtils contract swap function will revert for Uniswap v2 swap

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/utils/VeloSwapUtils.sol#L22-L41

Tool used

Manual Review

Recommendation

Pass IVeloRouter.Route[] route argument for Uniswap V2 swap execution, instead of simply array of addreses

MirthFutures commented 2 months ago

Info bug as the function in the library is not used in the strategy.