sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

BAICE - Mismatch length of destinations and amounts's length , will cause telcoin distribution fail #186

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

BAICE

medium

Mismatch length of destinations and amounts's length , will cause telcoin distribution fail

Summary

No checks of array length of telcoin transfer destinations and amounts .

Vulnerability Detail

In TelcoinDistributor:executeTransaction, when several require() statements are passed, the batchTelcoin will be executed , but there is no checks of destinations's length and amounts matchness .

Impact

Batch telcoin transfers will fail .

Code Snippet

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

some requires checks before telcoin transfer .

     // Makes sure the id exists
        require(
            transactionId < proposedTransactions.length,
            "TelcoinDistributor: Invalid index"
        );
        // Reverts if the challenge period has not expired
        require(
            block.timestamp >
                proposedTransactions[transactionId].timestamp + challengePeriod,
            "TelcoinDistributor: Challenge period has not ended"
        );
        // makes sure the transaction was not challenged
        require(
            !proposedTransactions[transactionId].challenged,
            "TelcoinDistributor: transaction has been challenged"
        );
        // makes sure the transaction was not executed previously
        require(
            !proposedTransactions[transactionId].executed,
            "TelcoinDistributor: transaction has been previously executed"
        );

Telcoin transfer execution .

   uint256 initialBalance = TELCOIN.balanceOf(address(this));
        //transfers amounts
        TELCOIN.safeTransferFrom(owner(), address(this), totalWithdrawl);
        for (uint i = 0; i < destinations.length; i++) {
            TELCOIN.safeTransfer(destinations[i], amounts[i]);
        }

Tool used

Manual Review, VScode

Recommendation

Add a new require statement to check destinations's length is equals to amounts matchness .

@+    require( proposedTransactions[transactionId].destinations == proposedTransactions[transactionId].amounts, "TelcoinDistributor: destinations and amounts must be equal")

Duplicate of #2

sherlock-admin2 commented 6 months ago

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

takarez commented:

invalid because {This is invalid because attempting to call a function with mismatched array can be considered a mallicious act and thus the expected result is to revert; but for an innocent user that dosnt know it can be considerd a valid due to revert of a called funcion but i believe for innocent users the front-end will solve this issue and make sure they put the right match}