Behavior can be changed If the block.number exceeds to 32 bit

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L1054

Vulnerability details

Impact

During the code review, It has been observed that _writeCheckPoint function is missing check when the block.number exceeds to 32 bits. The necessary checks are not implemented on the function.

Proof of Concept

1. Navigate to the following contract.

    function _writeCheckpoint(address delegatee, uint256 newVotes) internal {
        // write a new checkpoint for an user
        uint pos = checkpoints[delegatee].length;

        if (pos > 0 && checkpoints[delegatee][pos - 1].fromBlock == block.number) {
            checkpoints[delegatee][pos - 1].votes = safe224(newVotes);
        } else {
            uint32 blockNumber = safe32(block.number);
            checkpoints[delegatee].push(Checkpoint(blockNumber, safe224(newVotes)));
        }
    }

2. The check for the safe32(block.number) is missing on the function.

Tools Used

Code Review

Recommended Mitigation Steps

It is recommended to add the following check at the beginning of the function..

            safe32(block.number, "Paladin::_writeCheckpoint: block number exceeds 32 bits");

[Kogaroshi]

The check for safe32 is made in the function: uint32 blockNumber = safe32(block.number); (as shown in the Issue PoC), that will use the contract safe32() method do perform the correct checks, and revert if the block number exceeds uint32 (in something like 1700 years ?) safe32() method: https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L1387

[0xean]

closing as invalid. Worth noting that if the system is deployed to an EVM with much more frequent blocks this could become problematic sooner than 1700 years....