swapAVAXForExactTokens() Logical error

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-traderjoe/blob/main/src/LBRouter.sol#L520

Vulnerability details

Impact

In the last line, the excess money is returned to the user. But the processing is reversed amountsIn[0] -msg.value

function swapAVAXForExactTokens(
    uint256 _amountOut,
    uint256[] memory _pairBinSteps,
    IERC20[] memory _tokenPath,
    address _to,
    uint256 _deadline
)
    external
    payable
    override
    ensure(_deadline)
    verifyInputs(_pairBinSteps, _tokenPath)
    returns (uint256[] memory amountsIn)
{
    if (_tokenPath[0] != IERC20(wavax)) revert LBRouter__InvalidTokenPath(address(_tokenPath[0]));

    address[] memory _pairs = _getPairs(_pairBinSteps, _tokenPath);
    amountsIn = _getAmountsIn(_pairBinSteps, _pairs, _tokenPath, _amountOut);

    if (amountsIn[0] > msg.value) revert LBRouter__MaxAmountInExceeded(msg.value, amountsIn[0]);

    _wavaxDepositAndTransfer(_pairs[0], amountsIn[0]);

    uint256 _amountOutReal = _swapTokensForExactTokens(_pairs, _pairBinSteps, _tokenPath, amountsIn, _to);

    if (_amountOutReal < _amountOut) revert LBRouter__InsufficientAmountOut(_amountOut, _amountOutReal);

    if (msg.value > amountsIn[0]) _safeTransferAVAX(_to, amountsIn[0] - msg.value); // @audit 
}

Proof of Concept

forge test --mt testswapAVAXForExactTokens -vvvv

function testswapAVAXForExactTokens() public {
    uint256 amountOut = 1e18;

    (uint256 amountIn, ) = router.getSwapIn(pairWavax, amountOut, false);

    IERC20[] memory tokenList = new IERC20[](2);
    tokenList[0] = wavax;
    tokenList[1] = token6D;
    uint256[] memory pairVersions = new uint256[](1);
    pairVersions[0] = DEFAULT_BIN_STEP;

    vm.deal(DEV, amountIn*2);
    router.swapAVAXForExactTokens{value: amountIn+1}(amountOut, pairVersions, tokenList, DEV, block.timestamp);

    assertApproxEqAbs(token6D.balanceOf(DEV), amountOut, 13);
}

Tools Used

forge

Recommended Mitigation Steps

if (msg.value > amountsIn[0]) _safeTransferAVAX(_to,msg.value - amountsIn[0]);

[trust1995]

This is very far from critical vulnerability, the worse that can happen is that the TX through the router will revert if the sent amount is too high. Overly inflated.

[GalloDaSballo]

Dup of #469

[GalloDaSballo]

L

[c4-judge]

Duplicate of #124

[Simon-Busch]

Reverted to M as requested by @GalloDaSballo
Duplicate of #469

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo marked the issue as duplicate of #469

[Simon-Busch]

Marked this issue as satisfactory as requested by @GalloDaSballo