sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

sonny2k - Missing check if the sum of amounts value in array equals to totalWithdrawl in proposeTransaction() #237

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

sonny2k

medium

Missing check if the sum of amounts value in array equals to totalWithdrawl in proposeTransaction()

Summary

The proposeTransaction() functions in TelcoinDistributor.sol does not check whether the sum of amounts and totalWithdrawl are equal. This can lead to unexpected results.

Vulnerability Detail

In the proposeTransaction() function, the council member submits the array of amounts value which will be later sent and a value of totalWithdrawl as a representative of the amounts value summary.

However, in this functions, there is no check if the total value of these amounts equal totalWithdrawl

Impact

If the sum of amounts is smaller or larger than totalWithdrawl, the proposed transaction will always be reverted caused by the logic of batchTelcoin():

        require(
            TELCOIN.balanceOf(address(this)) == initialBalance,
            "TelcoinDistributor: must not have leftovers"
        );

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 proposed transaction () function that confirms that total value of amounts, and totalWithdrawl are all equal.

+ uint256 totalAmount;
+ for (uint256 i = 0; i <= amounts.length; i++) {
+             totalAmount += amounts[i]
+ {
+ require(totalWithdrawl == totalAmount, "mismatched array lengths");

Duplicate of #91

sherlock-admin2 commented 5 months ago

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

takarez commented:

invalid because { user are expected to specify the intended amount of all the destinations(totalWithdraw) the result of putting a larger or lower number than your commulative intention is to revert; but for making it robust function ; it should be considerd valid and a low or a requesting-feature issue which is invalid according to sherlock rules}