The Loop implementation is wrong . Loop always remove only first operator because ``address operator = operatorsForTokenId.at(0)`` wrong implementation

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol#L275

Vulnerability details

Impact

address operator = operatorsForTokenId.at(0) statement will always return the first operator in the list. So as per current implementation only possible to remove first operator. use i instead of 0 in loop iterations. When we use i this will fetch all values and remove all operators.

Proof of Concept

FILE: Breadcrumbs2023-06-lukso/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol

    function _clearOperators(
        address tokenOwner,
        bytes32 tokenId
    ) internal virtual {
        // here is a good example of why having multiple operators will be expensive.. we
        // need to clear them on token transfer
        //
        // NOTE: this may cause a tx to fail if there is too many operators to clear, in which case
        // the tokenOwner needs to call `revokeOperator` until there is less operators to clear and
        // the desired `transfer` or `burn` call can succeed.
        EnumerableSet.AddressSet storage operatorsForTokenId = _operators[
            tokenId
        ];

        uint256 operatorListLength = operatorsForTokenId.length();
        for (uint256 i = 0; i < operatorListLength; ) {
            // we are emptying the list, always remove from index 0
            address operator = operatorsForTokenId.at(0); //@audit always return first operator in this list . ``i`` should be used instead of ``0``
            _revokeOperator(operator, tokenOwner, tokenId);

            unchecked {
                ++i;
            }
        }
    }

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol#L258-L282

Tools Used

Manual Audit

Recommended Mitigation Steps

Use i instead of 0 in loop execution

  address operator = operatorsForTokenId.at(i);

Assessed type

Loop

[c4-pre-sort]

minhquanym marked the issue as duplicate of #33

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid