sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Jaraxxus - The variables of proposeTransactions are not checked properly #184

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

Jaraxxus

medium

The variables of proposeTransactions are not checked properly

Summary

Telcoin Distributor contract proposeTransaction() does not check the totalWithdrawal, destinations and amount properly, which will affect the actual proposal when it comes to execution.

Vulnerability Detail

proposeTransaction() simply pushes the array into the propsedTransactions array without checking the variables.

  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
            })
        );

When it comes to execution, the sum of the amount must be equal to the total withdrawal. Also, the length of the destinations array should be equal to the length of the amounts array as well.

Since these variables are not checked thoroughly, after it passes the challenge period, the execution will be reverted.

Impact

The execution will be reverted after challenge period, which wastes time.

Code Snippet

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

Tool used

Manual Review

Recommendation

Recommend checking that totalWithdrawal == all the amounts and that the length of the destinations == amounts in the proposeTransaction funciton.

Duplicate of #91

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 as its telling the user that something is wrong for him to set the correct values correctly; revert that are accepted are those when everything has been entered correctly and by legitimate innocent users;this is invalid!!!}