SwapAction::getSwapToken will return wrong swap token for balancer EXACT_OUT swaps

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/proxy/SwapAction.sol#L320-L330

Vulnerability details

Impact

As known, when doing a Balancer EXACT_OUT batch swap, assets should be passed in reverse order, this is thoroughly documented here.

Swapping in USDC for an exact amount out of BAL
swapType = EXACT_OUT and assets = [BAL, WETH, DAI, USDC]:

However, in SwapAction::getSwapToken, for Balancer swaps it always returns the last asset in the assets array, which is correct for EXACT_IN but wrong for EXACT_OUT, where it should be the first asset in the assets array.

Proof of Concept

function test_wrongSwapTokenReturned() public {
    uint256 amount = 200 ether;

    deal(address(token), user, amount);
    deal(address(underlyingToken), user, amount);

    address[] memory assets = new address[](2);
    assets[0] = address(underlyingToken);
    assets[1] = address(token);

    SwapParams memory swapParams = SwapParams({
        swapProtocol: SwapProtocol.BALANCER,
        swapType: SwapType.EXACT_OUT,
        assetIn: address(token),
        amount: 1 ether,
        limit: 2 ether,
        recipient: user,
        deadline: block.timestamp,
        args: abi.encode(weightedPoolIdArray, assets)
    });

    vm.startPrank(user);

    token.approve(address(swapAction), amount);
    underlyingToken.approve(address(swapAction), amount);

    uint256 tokenBalanceBefore = token.balanceOf(address(user));
    uint256 underlyingTokenBalanceBefore = underlyingToken.balanceOf(address(user));

    // Swap token for underlying token, using EXACT_OUT
    swapAction.transferAndSwap(user, emptyPermitParams, swapParams);

    uint256 tokenBalanceAfter = token.balanceOf(address(user));
    uint256 underlyingTokenBalanceAfter = underlyingToken.balanceOf(address(user));

    // Verify that the out token is the underlying token
    assertLt(tokenBalanceAfter, tokenBalanceBefore);
    assertGt(underlyingTokenBalanceAfter, underlyingTokenBalanceBefore);

    // `getSwapToken` returns the out token as `token` which is wrong
    assertEq(swapAction.getSwapToken(swapParams), address(token));

    vm.stopPrank();
}

Tools Used

Manual review

Recommended Mitigation Steps

Add the following in SwapAction::getSwapToken:

if (swapParams.swapType == SwapType.EXACT_OUT) token = primarySwapPath[0];
else token = primarySwapPath[primarySwapPath.length - 1];

Assessed type

Error

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report