Improper design/implementation of `SingleTokenJoinV2#joinTokenSingle()` make it prone to fail

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-12-amun/blob/98f6e2ff91f5fcebc0489f5871183566feaec307/contracts/basket/contracts/facets/Basket/BasketFacet.sol#L160-L183

for (uint256 i; i < bs.tokens.length; i++) {
    IERC20 token = bs.tokens[i];
    uint256 tokenAmount =
        balance(address(token)).mul(_amount.add(feeAmount)).div(
            totalSupply
        );
    require(tokenAmount != 0, "AMOUNT_TOO_SMALL");
    token.safeTransferFrom(msg.sender, address(this), tokenAmount);
}

// If there is any fee that should go to the beneficiary mint it
if (
    feeAmount != 0 &&
    bs.entryFeeBeneficiaryShare != 0 &&
    bs.feeBeneficiary != address(0)
) {
    uint256 feeBeneficiaryShare =
        feeAmount.mul(bs.entryFeeBeneficiaryShare).div(10**18);
    if (feeBeneficiaryShare != 0) {
        LibERC20.mint(bs.feeBeneficiary, feeBeneficiaryShare);
    }
}

LibERC20.mint(msg.sender, _amount);

In the current implementation, when entryFeeBeneficiaryShare is set to less than 100%, every new joinPool() will cause amounts of underlying tokens for each basketToken to increase.

Therefore, when a new pool is open for investment, or when the network is congested, it's very likely that by the time the joinPool() gets packed into the block, the amounts of underlying tokens required to get a certain amount of basketToken will be higher.

However, since SingleTokenJoinV2#joinTokenSingle() only swap for the exact amounts of underlying tokens specified in calldata, when the situation described above happens, IBasketFacet#joinPool() will fail due to insufficient balance of underlying tokens.

https://github.com/code-423n4/2021-12-amun/blob/98f6e2ff91f5fcebc0489f5871183566feaec307/contracts/basket/contracts/singleJoinExit/SingleTokenJoinV2.sol#L86-L112

for (uint256 i; i < _joinTokenStruct.trades.length; i++) {
    UnderlyingTrade calldata trade = _joinTokenStruct.trades[i];
    uint256[] memory inputs = new uint256[](trade.swaps.length + 1);
    inputs[0] = trade.quantity;
    //Get inputs to
    for (uint256 j; j < trade.swaps.length; j++) {
        UniswapV2SwapStruct calldata swap = trade.swaps[
            trade.swaps.length - j - 1
        ];
        uint256[] memory amounts = IPangolinRouter(swap.exchange)
            .getAmountsIn(inputs[j], swap.path);
        inputs[j + 1] = amounts[0];
    }

    for (uint256 j; j < trade.swaps.length; j++) {
        UniswapV2SwapStruct calldata swap = trade.swaps[j];
        uint256 amountIn = inputs[trade.swaps.length - j];
        _maxApprove(IERC20(swap.path[0]), address(swap.exchange));
        IPangolinRouter(swap.exchange).swapExactTokensForTokens(
            amountIn,
            0,
            swap.path,
            address(this),
            block.timestamp
        );
    }
}

Recomandation

Consider adding a new method or parameter to allow joinPool() with minAmountOut.

[loki-sama]

Duplicate #172

[0xleastwood]

Agree, this sounds like a similar issue raised in #212. So marking this invalid in favour of the same warden's duplicated issue.