Function spec and implementation difference / strict comparison

[code423n4]

Handle

loop

Vulnerability details

The function tokenOfOwnerByIndex has a requirement for _index to be 0. This is different from the specification, which states:

/// @dev Throws if `_index` >= `balanceOf(_keyOwner)` or if
///  `_keyOwner` is the zero address, representing invalid NFTs.

It also seems to be a bit unnecessary as _index is not even used in the following call getTokenIdFor(_keyOwner).

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other https://github.com/code-423n4/2021-11-unlock/blob/main/smart-contracts/contracts/mixins/MixinERC721Enumerable.sol#L44-L59

Recommended Mitigation Steps

Change require statement to something according to specification:
require(_index < balanceOf(_keyOwner) && _keyOwner != address(0));

[julien51]

This is indeed an issue! Let's get it fixed @clemsos