In `LSP6SetDataModule.sol`, Infinite loop in `_verifyAllowedERC725YDataKeys()`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol#L726-L734

Vulnerability details

Impact

In _verifyAllowedERC725YDataKeys() there is a great possibility of an Infinite loop. This is because ++ii is an increment inside if condition. This can lead to excessive gas consumption, causing the Ethereum transaction to fail due to the gas limit

Proof of Concept

In the loop of the _verifyAllowedERC725YDataKeys() method, the position of ++i is wrong, which may lead to a infinite loop

File: LSP6SetDataModule.sol
622:    function _verifyAllowedERC725YDataKeys(
----
726:            for (uint256 ii; ii < inputKeysLength; ) {
727:                // if the input data key has been marked as allowed previously,
728:                // SKIP it and move to the next input data key.
729:                if (validatedInputKeysList[ii]) {
730:                    unchecked {
731:                        ++ii; // @audit possibility of an Infinite loop
732:                    }
733:                    continue;
734:                }

In the above code, when !validatedInputKeysList[ii], ii is not incremented, resulting in a infinite loop.

Tools Used

Manual Review

Recommended Mitigation Steps

To avoid this potential infinite loop, move the unchecked box outside the if condition.

Assessed type

DoS

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-pre-sort]

minhquanym marked the issue as low quality report

[minhquanym]

Seems invalid as ii is increased at the end

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid