safeInflationBatchTransferFrom does not perform array length checks

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xb77ca2cee19b08da466c23477ba0753e7722b3ad28a5ce2b2e0fae125525bf65 Severity: medium

Description: When working with address and amount arrays, it is common practice to check that the arrays are of equal length. This prevents issues such as mismatched transfers, where addresses could receive incorrect amounts or, in the worst case, nothing at all.

This happens everywhere else in the code, for example, inside Migration:Migrate:

    if (_avatars.length != _amounts.length) {
        revert CirclesArraysLengthMismatch(_avatars.length, _amounts.length, 0);

Now, let’s take a closer look at the function safeInflationBatchTransferFrom:

    function safeInflationaryBatchTransferFrom(
        address _from,
        address _to,
        uint256[] memory _ids,
        uint256[] memory _inflationaryValues,
        bytes memory _data
    ) public OnlyActOnBalancesOfSender(_from) {
        uint256[] memory values = convertBatchInflationaryToDemurrageValues(_inflationaryValues, day(block.timestamp));
        hub.safeBatchTransferFrom(_from, _to, _ids, values, _data);
    }

This function accepts two arrays, _ids and _inflationaryValues. Logically, there should be a check to ensure both arrays are of the same length, confirming they are correctly paired. However, this check is missing in the function's flow.

As previously mentioned, this oversight can lead to significant issues. For example, if the arrays are mismatched, certain _ids may not receive their corresponding _inflationaryValue (if the values array is longer than the _ids array).

This issue should be classified as medium severity due to its potential to:

• Temporarily make essential contract functionality unusable or inaccessible.

Since this function may result in incorrect transfers or prevent recipients from receiving their intended amounts, it can make an essential functionality (batch transferring inflationary values), temporarily (even if for just 1 transaction) unusable (it does not work as it should).

Recommendation

Ensure that a check is added to verify that the length of _ids is equal to the length of _inflationaryValues.

[benjaminbollen]

Thank you for your report on the safeInflationBatchTransferFrom function. After review, we've determined this is not an issue.

The function does not require an explicit array length check because this validation is already performed in the ERC1155:_update() function, which is called deeper in the stack. This design ensures array length consistency without redundant checks.

We appreciate your careful examination of our code and your focus on input validation. Your diligence helps maintain the robustness of our system. Thank you for your valuable contribution.