araj - Improper input validation in `telcoinDistributor::proposeTransaction`

[sherlock-admin2]

araj

medium

Improper input validation in telcoinDistributor::proposeTransaction

Summary

A transaction can be created with different length of destinations and amount, if that happens it will revert while executing with out-of-bound error

Vulnerability Detail

telcoinDistributor::proposeTransaction is taking destinations and amounts array as inputs, there can be a scenario where there lengths are not same as there is no checks, if that happens then transaction will revert even after it has no challenge because of out-of-bound error

    function proposeTransaction(
        uint256 totalWithdrawl,
        address[] memory destinations,
        uint256[] memory amounts
    ) external onlyCouncilMember whenNotPaused {
        // Pushing the proposed transaction to the array
        proposedTransactions.push(
            ProposedTransaction({
                totalWithdrawl: totalWithdrawl,
                destinations: destinations,
                amounts: amounts,
                timestamp: uint64(block.timestamp),
                challenged: false,
                executed: false
            })
        );

        // Emitting an event after proposing a transaction
        emit TransactionProposed(proposedTransactions.length - 1, _msgSender());
    }

Impact

Transaction will revert even after having no challenge

Code Snippet

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

Tool used

Manual Review

Recommendation

Use checks in proposeTransaction()

+  require(destinations.length == amounts.length && destinations.length > 0 && amounts.length > 0, "TelcoinDistributor: Invalid length");

Duplicate of #2

[sherlock-admin2]

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

takarez commented:

invalid because { this is invalid because of how the watson explain; had he sais it will kead to unextepected behavior;it then will fall under a dupp of 001 or 002; but reverting in this case is a good thing to tell a user that something is wrong; so i consider it invalid due to lack of clear explanation}