Summary
The veRAACToken::withdraw() function lacks an implementation to update _lockState.totalLocked, leading to an unchecked increase in its value. Over time, this results in an overflow, preventing the contract from locking new tokens and disrupting its intended functionality.
Vulnerability Details
The functions veRAACToken::lock() and veRAACToken::increase() correctly update _lockState.totalLocked when executed. However, the veRAACToken::withdraw() function only clears user-specific data and does not adjust _lockState.totalLocked. This omission results in a continuously increasing value, eventually leading to an overflow.
The current implementation of withdraw() lacks the necessary update to _lockState.totalLocked:
function withdraw() external nonReentrant {
LockManager.Lock memory userLock = _lockState.locks[msg.sender];
if (userLock.amount == 0) revert LockNotFound();
if (block.timestamp < userLock.end) revert LockNotExpired();
uint256 amount = userLock.amount;
uint256 currentPower = balanceOf(msg.sender);
@> delete _lockState.locks[msg.sender];
@> delete _votingState.points[msg.sender];
_checkpointState.writeCheckpoint(msg.sender, 0);
_burn(msg.sender, currentPower);
raacToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
}
Since _lockState.totalLocked is never updated upon withdrawal, its value continues to grow indefinitely.
Poc
Add the following test to test/unit/core/tokens/veRAACToken.test.js and execute it:
describe("withdraw fails to update totalLocked", () => {
it("Poc", async () => {
const amount = ethers.parseEther("2000");
const duration = 365 * 24 * 3600 * 4 ;
await veRAACToken.connect(users[0]).lock(amount, duration);
const boostStateOne = await veRAACToken.getBoostState();
console.log("first _lockState.totalLocked:",boostStateOne.totalWeight);
await time.increase(365 * 24 * 3600 * 4 + 100);
await network.provider.send("evm_mine");
await veRAACToken.connect(users[0]).withdraw();
await veRAACToken.connect(users[1]).lock(ethers.parseEther("2000"), duration);
const boostStateTwo = await veRAACToken.getBoostState();
console.log("second _lockState.totalLocked:",boostStateTwo.totalWeight);
await time.increase(365 * 24 * 3600 * 4 + 100);
await network.provider.send("evm_mine");
await veRAACToken.connect(users[1]).withdraw();
await network.provider.send("evm_mine");
await veRAACToken.connect(users[1]).lock(ethers.parseEther("1"), duration);
const boostStateThree = await veRAACToken.getBoostState();
console.log("third _lockState.totalLocked:",boostStateThree.totalWeight);
console.log("total voting power:", await veRAACToken.getTotalVotingPower());
});
});
output:
veRAACToken
withdraw fails to update totalLocked
first _lockState.totalLocked: 2000000000000000000000n
second _lockState.totalLocked: 4000000000000000000000n
third _lockState.totalLocked: 4001000000000000000000n
total voting power: 1000000000000000000n
From the output, we observe that _lockState.totalLocked continuously increases, deviating significantly from the expected total voting power. Due to this unbounded growth, the value will eventually overflow, causing the contract to malfunction.
Impact
The unchecked expansion of _lockState.totalLocked will ultimately result in an overflow, preventing the contract from functioning correctly. Once the overflow occurs, new tokens can no longer be locked, rendering the contract unusable.
Tools Used
Manual Review
Recommendations
To resolve this issue, update _lockState.totalLocked in the withdraw() function by subtracting the withdrawn amount.For example:
function withdraw() external nonReentrant {
LockManager.Lock memory userLock = _lockState.locks[msg.sender];
if (userLock.amount == 0) revert LockNotFound();
if (block.timestamp < userLock.end) revert LockNotExpired();
uint256 amount = userLock.amount;
uint256 currentPower = balanceOf(msg.sender);
// Update totalLocked
+ _lockState.totalLocked -= amount;
// Clear lock data
delete _lockState.locks[msg.sender];
delete _votingState.points[msg.sender];
// Update checkpoints
_checkpointState.writeCheckpoint(msg.sender, 0);
// Burn veTokens and transfer RAAC
_burn(msg.sender, currentPower);
raacToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
}
test again:
veRAACToken
withdraw miss update totalLocked
first _lockState.totalLocked: 2000000000000000000000n
second _lockState.totalLocked: 2000000000000000000000n
third _lockState.totalLocked: 1000000000000000000n
total voting power: 1000000000000000000n
