TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Inaccurate Timing Due to Use of `block.number` on Arbitrum

boredpukar

Summary

The TempleGoldStaking contract employs `block.number` to manage delays and timing within its functions. However, on Arbitrum, `block.number` does not reliably reflect real-time passage due to its synchronization with L1 block numbers only once per minute. This discrepancy can lead to significant vulnerabilities, allowing users to exploit timing-based protections in the contract.

Vulnerability Details

Functions Affected:

  1. getPriorVotes(address account, uint256 blockNumber)

  2. _writeCheckpoint(address delegatee, uint256 nCheckpoints, uint256 oldVotes, uint256 newVotes)

These functions rely on block.number to determine the timing and validity of voting power and checkpoint records. On Arbitrum, block.number reflects the L1 block number, which is updated approximately once per minute. This means that within a 60-second window, the block number remains constant, then jumps to the latest L1 block number, and repeats this process.

A user can exploit this by timing their transactions to occur just before and after the block number synchronization. For instance, they can open a position one Arbitrum block before the sync and close it immediately after. This would make it appear that there has been a significant delay (e.g., 5 blocks, considering an average mainnet block time of 12 seconds) when, in reality, only one Arbitrum block has passed. This discrepancy could be used to bypass timing restrictions within the contract.

Detailed Explaination: Block gas limit, numbers and time

function getPriorVotes(address account, uint256 blockNumber) external override view returns (uint256) {
if (blockNumber >= block.number) { revert InvalidBlockNumber(); }
​
uint256 nCheckpoints = numCheckpoints[account];
if (nCheckpoints == 0) {
return 0;
}
​
// First check most recent balance
if (_checkpoints[account][nCheckpoints - 1].fromBlock <= blockNumber) {
return _checkpoints[account][nCheckpoints - 1].votes;
}
​
// Next check implicit zero balance
if (_checkpoints[account][0].fromBlock > blockNumber) {
return 0;
}
​
uint256 lower = 0;
uint256 upper = nCheckpoints - 1;
while (upper > lower) {
uint256 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
Checkpoint memory cp = _checkpoints[account][center];
if (cp.fromBlock == blockNumber) {
return cp.votes;
} else if (cp.fromBlock < blockNumber) {
lower = center;
} else {
upper = center - 1;
}
}
return _checkpoints[account][lower].votes;
}
function _writeCheckpoint(
address delegatee,
uint256 nCheckpoints,
uint256 oldVotes,
uint256 newVotes
) internal {
if (nCheckpoints > 0 && _checkpoints[delegatee][nCheckpoints - 1].fromBlock == block.number) {
_checkpoints[delegatee][nCheckpoints - 1].votes = newVotes;
} else {
_checkpoints[delegatee][nCheckpoints] = Checkpoint(block.number, newVotes);
numCheckpoints[delegatee] = nCheckpoints + 1;
}
emit DelegateVotesChanged(delegatee, oldVotes, newVotes);
}

Impact

These are critical functions and they may not always work correctly as intended.

Tools Used

Manual Review

Recommendations

Replace the use of block.number with block.timestamp for reliable timing calculations. Unlike block.number, block.timestamp reflects the actual passage of time and is updated consistently.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.