Chom - UniswapV2 has swapExactETHForTokens which takes native ETH but why you are saying msgValue is always zero for UniswapV2?

[sherlock-admin]

Chom

medium

UniswapV2 has swapExactETHForTokens which takes native ETH but why you are saying msgValue is always zero for UniswapV2?

Summary

UniswapV2 has swapExactETHForTokens which takes native ETH but why you are saying msgValue is always zero for UniswapV2?

Vulnerability Detail

    function getExecutionData(address from, Trade calldata trade)
        internal view returns (
            address spender,
            address target,
            uint256 /* msgValue */,
            bytes memory executionCallData
        )
    {
        TradeType tradeType = trade.tradeType;
        UniV2Data memory data = abi.decode(trade.exchangeData, (UniV2Data));

        spender = address(Deployments.UNIV2_ROUTER);
        target = address(Deployments.UNIV2_ROUTER);
        // msgValue is always zero for uniswap

        if (
            tradeType == TradeType.EXACT_IN_SINGLE ||
            tradeType == TradeType.EXACT_IN_BATCH
        ) {
            executionCallData = abi.encodeWithSelector(
                IUniV2Router2.swapExactTokensForTokens.selector,
                trade.amount,
                trade.limit,
                data.path,
                from,
                trade.deadline
            );
        } else if (
            tradeType == TradeType.EXACT_OUT_SINGLE ||
            tradeType == TradeType.EXACT_OUT_BATCH
        ) {
            executionCallData = abi.encodeWithSelector(
                IUniV2Router2.swapTokensForExactTokens.selector,
                trade.amount,
                trade.limit,
                data.path,
                from,
                trade.deadline
            );
        }
    }

You are assuming "msgValue is always zero for uniswap" and ignore the case that msgValue is positive.

If msgValue is positive, you must use swapExactETHForTokens / swapETHForExactTokens instead of swapExactTokensForTokens / swapTokensForExactTokens.

Impact

UniswapV2 support swapping native ETH for other tokens but you said that "msgValue is always zero" which is assuming UniswapV2 can't do it. You will be losing a feature. And if input with positive msgValue, unexpected behavior may be executed on UniswapV2 too such as swapping WETH to the token instead of using native ETH.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/UniV2Adapter.sol#L12-L52

Tool used

Manual Review

Recommendation

Handle positive msgValue for UniswapV2 using swapExactETHForTokens / swapETHForExactTokens