Infinite loop if there are too many members to add

[cedoor]

Describe the bug

The Semaphore contract allows the admin to add group members in a batch by calling addMembers (presumably for gas efficiency). When this function is called, a for loop iterates over all identityCommitments upon which _addMember is called. This for loop, however, uses a uint8 as the iterator and makes the iterator increment unchecked. As a result, if the input array’s size is larger than 255 (the maximum value of a uint8) then the iterator’s value will overflow causing the loop to restart at 0 resulting in an infinite loop.

Impact

If an admin adds more than 255 members, the infinite loop will consume all of the transaction’s gas and then revert. This therefore can waste a user’s funds.

Additional context

This bug was found by Veridise during their audit of Semaphore. If you acknowledge and fix this bug, can you please mention Veridise in the commit.