0xC - Potential overflow in `TelcoinDistributor` contract's `proposeTransaction` function

[sherlock-admin2]

0xC

medium

Potential overflow in TelcoinDistributor contract's proposeTransaction function

Summary

The TelcoinDistributor smart contract, as presented, contains a potential vulnerability in the assignment of the timestamp variable. The assignment of block.timestamp to timestamp without proper validation can lead to an overflow if block.timestamp exceeds the maximum value that a uint64 can represent.

Vulnerability Detail

In the TelcoinDistributor contract's proposeTransaction function, the timestamp variable is assigned the value of block.timestamp without any validation. The timestamp variable is of type uint64, which can represent values from 0 to 2^64-1. However, if block.timestamp exceeds this maximum value, an overflow will occur, resulting in an incorrect timestamp value.

As there is no check to ensure that block.timestamp does not exceed the maximum value representable by a uint64, it is possible for an overflow to occur, leading to incorrect timestamps in the ProposedTransaction struct.

Impact

The potential overflow in the assignment of the timestamp variable can have a significant impact on the TelcoinDistributor contract. If an overflow occurs, the timestamps recorded for proposed transactions will be incorrect, which can lead to various issues, including challenges being initiated or executed at unexpected times. This can disrupt the intended operation of the contract and compromise its functionality.

Code Snippet

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

Tool used

Manual Review

Recommendation

To mitigate the potential overflow vulnerability, it is recommended to add a validation check to ensure that block.timestamp does not exceed the maximum value representable by a uint64 before assigning it to the timestamp variable or just to use uint256 instead of uint64 for the timestamp variable.

Here is the updated code snippet with the recommended validation check:

// Ensure that block.timestamp does not exceed the maximum value representable by uint64

    function proposeTransaction(
        uint256 totalWithdrawl,
        address[] memory destinations,
        uint256[] memory amounts
    ) external onlyCouncilMember whenNotPaused {
+       require(block.timestamp <= type(uint64).max, "TelcoinDistributor: Timestamp overflow");
        // Pushing the proposed transaction to the array
        proposedTransactions.push(
            ProposedTransaction({
                totalWithdrawl: totalWithdrawl,
                destinations: destinations,
                amounts: amounts,
                timestamp: uint64(block.timestamp),   // H Potential overflow
                challenged: false,
                executed: false
            })
        );

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

By implementing this change, the contract will validate that block.timestamp does not exceed the maximum value representable by a uint64 before recording it as the timestamp for proposed transactions, preventing potential overflow issues.

[sherlock-admin2]

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

takarez commented:

invalid because { This is invalid as the timestamp need to reach a whooping 18446744073709551615 to overflow which is a lot of years}

[nevillehuang]

Invalid, this overflow will only occur on Sun Jul 21 2554, which is basically a non issue