Function using msg.value called in loop

[code423n4]

Handle

cmichel

Vulnerability details

The _transferInputTokens function accesses msg.value if the input token is ETH, and this function is called in a loop in _submitOutOrders.
Therefore, in theory, one can specify two ETH orders but only pay once as the msg.value will be processed in each order iteration.

Impact

The severity is low as

1. calls to _transferInputTokens in a loop always have _fromReserve == true which does not access msg.value
2. the excess ETH still needs to already exist in the contract, which should usually not be the case as the swap amounts are either sent to the reserve or to the caller.

Recommended Mitigation Steps

I'd advocate against using msg.value in a loop.
Imo, the best way to circumvent this is to not deal with ETH in the contract at all and just use WETH directly, as that's what the contract wants anyway for the orders.
Maybe just adding a depositAllETH function that deposits msg.value to WETH once at the beginning of the swap functions makes sense.

[maximebrugel]

We do not access msg.value in a loop expect multicall that we will remove (#226)