code-423n4 / 2021-12-amun-findings

0 stars 0 forks source link

Intermediate max-input-amount check in `_joinTokenSingle` is unnecessary #196

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

cmichel

Vulnerability details

The SingleTokenJoin._joinTokenSingle function computes an intermediate maxAmountIn value of amountsIn[0]:

function _joinTokenSingle(JoinTokenStruct calldata _joinTokenStruct)
        internal
{
    // ... swaps to intermediate first

    path[0] = address(INTERMEDIATE_TOKEN);

    _maxApprove(INTERMEDIATE_TOKEN, address(uniSwapLikeRouter));
    for (uint256 i; i < tokens.length; i++) {
        path[1] = tokens[i];

        uint256[] memory amountsIn = uniSwapLikeRouter.getAmountsIn(
            amounts[i],
            path
        );

        uniSwapLikeRouter.swapTokensForExactTokens(
            amounts[i],
            amountsIn[0], // @audit gas: just use MAX maxAmountIn
            path,
            address(this),
            _joinTokenStruct.deadline
        );
        _maxApprove(IERC20(tokens[i]), _joinTokenStruct.outputBasket);
    }

    IBasketFacet(_joinTokenStruct.outputBasket).joinPool(
        _joinTokenStruct.outputAmount,
        _joinTokenStruct.referral
    );

    // ######## SEND TOKEN #########

    uint256 outputAmount = outputToken.balanceOf(address(this));
    require(
        outputAmount == _joinTokenStruct.outputAmount,
        "FAILED_OUTPUT_AMOUNT"
    );

    outputToken.safeTransfer(msg.sender, outputAmount);
}

It's not required to compute the input amount that is needed for the exact output swap, just do the swap and use a maxAmountIn of type(uint256).max. The final min return check on the entire amount (outputAmount == _joinTokenStruct.outputAmount) is enough to guarantee no sandwiching.