sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

sonny2k - Missing check for equal length arrays in TelcoinDistributor: proposeTransaction() #228

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

sonny2k

medium

Missing check for equal length arrays in TelcoinDistributor: proposeTransaction()

Summary

The proposeTransaction() functions in TelcoinDistributor.sol does not check whether the lengths of the arrays submitted are equal. This can lead to unexpected results.

Vulnerability Detail

In the proposeTransaction() function, the council member submits two arrays (destinations, and amounts)

The expectation is that the council member submitting the function will ensure that the indexes of the arrays correspond to the correct values in the other arrays, thus that the lengths will be the same. Common practice in such a situation is to verify that the lengths are equal to ensure the council member hasn't made an error.

However, in this functions, we simply iterate through the destinations array without performing this check, and then call out safeTransfer() for each destination separately in batchTelcoin()

        for (uint i = 0; i < destinations.length; i++) {
            TELCOIN.safeTransfer(destinations[i], amounts[i]);
        }

Impact

If the destinations array is shorter than the amounts arrays, the additional values in the amounts arrays will be ignored. This could lead to transfers with unexpected results, while destinations array is longer than the amounts arrays, it would make the batchTelcoin() always revert since amounts[i] doesn't exist.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/protocol/core/TelcoinDistributor.sol#L87

Tool used

Manual Review

Recommendation

Add a check to the proposeTransaction() function that confirms that destinations, and amounts are all equal in length.

+ require(destinations.length == amounts .length, "mismatched array lengths");

Duplicate of #2

sherlock-admin2 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because {This is invalid because it will revert with leftover error msg; just like report 164 and 001 and 2}