Overflow in `depositMultiple` can lead to infinite loop and incorrect deposit

[code423n4]

Handle

hrkrshnn

Vulnerability details

Overflow in depositMultiple can lead to infinite loop and incorrect deposit

See depositMultiple function.

function depositMultiple(
    address[] calldata _tokens,
    uint256[] calldata _amounts
)
    external
    override
    notHalted
    returns (uint256 _shares)
{
    require(_tokens.length == _amounts.length, "!length");

    for (uint8 i; i < _amounts.length; i++) {
        _shares = _shares.add(deposit(_tokens[i], _amounts[i]));
    }
}

There are no checks that _amounts.length < 255 = type(uint8).max. This means that the following (hypothetical) situation is possible:

1. User wants to deposit the same token A 256 number of times.
2. User calls depositMultiple with _tokens = [address(A), address(A), ..., Address(A)] and _amounts = [// some numbers].
3. After i = 255, since i is of type uint8 and type(uint8).max = 255, i++ overflows to 0.

This will not only call deposit on the wrong index, but also leads to an infinite loop. Note that this problem exists regardless of the MAX token cap of 255. Because, technically one can deposit the same token more than 255 times (although it may not be logical to do so).

Recommended Mitigation Steps

• Either add a require(_tokens.length < 255) check
• Or, change uint8 i to uint i.

[Haz077]

Depositing same token multiple times is not an issue, #86 mentioned using uint256 instead of uint8 in loops which is more gas efficient.

[GalloDaSballo]

Duplicate of #86