[WP-H9] `_swapUniswapV2` may use an improper `path` which can cause a loss of the majority of the rewardTokens

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L311-L334

Vulnerability details

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L311-L334

function harvest(uint256 minOutCurve) external onlyRole(STRATEGIST_ROLE) {
    convexConfig.baseRewardPool.getReward(address(this), true);

    //Prevent `Stack too deep` errors
    {
        DexConfig memory dex = dexConfig;
        IERC20[] memory rewardTokens = strategyConfig.rewardTokens;
        IERC20 _weth = weth;
        for (uint256 i = 0; i < rewardTokens.length; i++) {
            uint256 balance = rewardTokens[i].balanceOf(address(this));

            if (balance > 0)
                //minOut is not needed here, we already have it on the Curve deposit
                _swapUniswapV2(
                    dex.uniswapV2,
                    rewardTokens[i],
                    _weth,
                    balance,
                    0
                );
        }

        uint256 wethBalance = _weth.balanceOf(address(this));
        require(wethBalance > 0, "NOOP");
        ...

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/strategies/StrategyPUSDConvex.sol#L410-L430

 function _swapUniswapV2(
    IUniswapV2Router router,
    IERC20 tokenIn,
    IERC20 tokenOut,
    uint256 amountIn,
    uint256 minOut
) internal {
    tokenIn.safeIncreaseAllowance(address(router), amountIn);

    address[] memory path = new address[](2);
    path[0] = address(tokenIn);
    path[1] = address(tokenOut);

    router.swapExactTokensForTokens(
        amountIn,
        minOut,
        path,
        address(this),
        block.timestamp
    );
}

In the current implementation, rewardTokens from the underlying strategy will be swapped to weth first then weth -> usdc.

However, the path used for swapping from rewardToken -> weth is hardcoded as [rewardToken, weth], which may not be the optimal route.

For example, the majority liquidity for a particular rewardToken may actually be in the rewardToken/USDC pool. Swapping through the rewardToken/WETH with low liquidity may end up getting only a dust amount of WETH.

Recommendation

Consider allowing the admin to set a path for the rewardTokens.

[spaghettieth]

As of now, the one in the contract is the optimal routing path.

[dmvt]

I think the warden has made a reasonable find and recommendation. The sponsor used the phrase 'as of now' in disputing the report, but the idea that it may not always be the optimal path is actually specifically what the report and its mitigation addresses. That said, external factors are required so moving it to medium severity.