Output token is obtained from the first trade in SingleNativeTokenExitV2.exit

[code423n4]

Handle

hyh

Vulnerability details

Impact

Exit swaps in each trade are restricted to (basket_token_i, ..., WETH) sequences as the output token is read from the first trade.
This way any other correct trade structures will fail the function because of the output token amount check.

Proof of Concept

SingleNativeTokenExitV2.exit assumes that all trades will have the same output token and reads the output token from the first trade:
https://github.com/code-423n4/2021-12-amun/blob/main/contracts/basket/contracts/singleJoinExit/SingleNativeTokenExitV2.sol#L93

When trades are more complicated than that due to liquidity or suitability considerations,
for example, user wants to swap all the basket tokens to WETH, then WETH to USDC and withdraw it,
or when basket is (t1, t2, WBTC) user may want to swap t1 to WBTC, t2 to WETH, then WBTC to WETH, and withdraw it,
current logic fails and function will be reverted because of the final output token amount check:
https://github.com/code-423n4/2021-12-amun/blob/main/contracts/basket/contracts/singleJoinExit/SingleNativeTokenExitV2.sol#L100

Recommended Mitigation Steps

Fix the index to locate the last token in the swap sequence:

Now:

address[] calldata path = _exitTokenStruct
		.trades[0]
		.swaps[_exitTokenStruct.trades[0].swaps.length - 1]
		.path;

To be:

uint tradesLastIndex_ = _exitTokenStruct.trades.length - 1;
address[] calldata path = _exitTokenStruct
		.trades[tradesLastIndex_]
		.swaps[_exitTokenStruct.trades[tradesLastIndex_].swaps.length - 1]
		.path;

Also, the comment looks like it is temporary, can it be clarified?
https://github.com/code-423n4/2021-12-amun/blob/main/contracts/basket/contracts/singleJoinExit/SingleNativeTokenExitV2.sol#L96

[loki-sama]

That is the intention of the contract to swap everything to a single token.

[0xleastwood]

I think this is a duplicate of #176