Submission Details
Severity:
The binary search in `TempleGoldStaking::getPriorVotes` has errors in center calculation, lower value update, and the while condition, leading to a Denial of Service (DoS) vulnerability.
## Relevant Github Links
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGoldStaking.sol#L231-L243
## Summary
The provided code for the binary search in `TempleGoldStaking::getPriorVotes` has three critical errors: the incorrect calculation of the `center` (which always results in zero for certain scenarios), an incorrect update of the `lower` value (leading to potential infinite loops or missed block numbers), and a faulty `while` condition that prevents reaching the maximum `upper` value (causing the search to stop prematurely). These errors can cause a Denial of Service (DoS) vulnerability by making it impossible to accurately determine the prior number of votes for an account at a specific block number.
## Vulnerability Details
Wrong mechanism for searching the specific blockNumber to search for the votes at spesific blockNumber checkpoint in `TempleGoldStaking::getPriorVotes`.
```javascript
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;
}
}
```
There are at least three issues in this mechanism that lead to errors:
#### 1. Wrong math to calculate the center between upper and lower:
```javascript
uint256 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
```
Lets have a scenario where:
* `nCheckpoints` is 5 and then `upper` is 4 (nCheckpoints - 1).
* `lower` is 0.
In this scenario, `center` will be `4 - (4 - 0) / 2` -> 0. `center` value will always be zero because basically upper will always be decremented with upper because lower is zero which will results zero.
#### 2. Wrong value of `lower` (Consider the first error has been fixed)
```javascript
} else if (cp.fromBlock < blockNumber) {
lower = center;
```
Lets have a scenario where:
* Checkpoint.fromBlock = [1,2,3,4,5].
* `blockNumber` is 5.
* `nCheckpoints` is 5 and then `upper` is 4 (nCheckpoints - 1).
* `lower` is 2.
* Second lower has been set to 3 (`center` is 3 (lower is 2, upper is 4))
* Third lower has been set to 3 (`center` is 3 (lower is 3, upper is 4)). The `center` will be 3 again because `(4 + 3) / 2 = 3.5` and it will be automatically rounded down.
* At the end, the intended blockNumber (4th epoch) will not be reached.
#### 3. Wrong while statement that causes max value in `upper (nCheckpoints - 1)` will not be reached. (Consider the first and second error has been fixed)
```javascript
while (upper > lower) {
```
Lets have a scenario where:
* Checkpoint.fromBlock = [1,2,3,4,5].
* `blockNumber` is 5.
* `nCheckpoints` is 5 and then `upper` is 4 (nCheckpoints - 1).
* `center` is 2.
* `lower` is 3.
* Second lower has been set to 3 (`center` is 3 (lower is 3, upper is 4)).
* Because of the wrong `while` statement. The calculation stops here and the intended blockNumber (4th epoch) will not be reached.
## Impact
This error causes a Denial Of Service (DoS) vulnerabilities to determine the prior number of votes for an account as of a block number. This vulnerabilities makes `TempleGoldStaking::getPriorVotes` only working for zero index epoch.
## Tools Used
Manual Review
## Recommendations
```diff
- while (upper > lower) {
+ while (upper >= lower) {
- uint256 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
+ uint256 center = 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;
+ lower = center + 1;
} else {
upper = center - 1;
}
}
```
Updates
Prize pool breakdown
Total prize
25,000 USDC
nSLOC
99825 USDC / LOC
20,000 USDC
2,500 USDC
2,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.