Summary
Users can overwrite their existing veRAACToken locks leading to loss of token when withdrawing due to a lack of check on existing lock.
Vulnerability Details
In order to participate in governance activities, users are required to lock their RAACToken for a minimum amount of time:
* @notice Creates a new lock position for RAAC tokens
* @dev Locks RAAC tokens for a specified duration and mints veRAAC tokens representing voting power
* @param amount The amount of RAAC tokens to lock
* @param duration The duration to lock tokens for, in seconds
*/
function lock(uint256 amount, uint256 duration) external nonReentrant whenNotPaused {
if (amount == 0) revert InvalidAmount();
if (amount > MAX_LOCK_AMOUNT) revert AmountExceedsLimit();
if (totalSupply() + amount > MAX_TOTAL_SUPPLY) revert TotalSupplyLimitExceeded();
if (duration < MIN_LOCK_DURATION || duration > MAX_LOCK_DURATION)
revert InvalidLockDuration();
raacToken.safeTransferFrom(msg.sender, address(this), amount);
uint256 unlockTime = block.timestamp + duration;
_lockState.createLock(msg.sender, amount, duration);
_updateBoostState(msg.sender, amount);
(int128 bias, int128 slope) = _votingState.calculateAndUpdatePower(
msg.sender,
amount,
unlockTime
);
uint256 newPower = uint256(uint128(bias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
_mint(msg.sender, newPower);
emit LockCreated(msg.sender, amount, unlockTime);
}
The problem is that users can creates new locks even when they have existing locks hence overwriting their records. This causes the contract to record wrong states including users lock amounts. Consider this POC that shows that a users can overwrite their lock and end-up withdrawing less than locked:
it("can lock twice", async () => {
const amount = ethers.parseEther("1000");
const duration = 365 * 24 * 3600;
const tx = await veRAACToken.connect(users[0]).lock(amount, duration);
await veRAACToken.connect(users[0]).lock(amount, duration);
const position = await veRAACToken.getLockPosition(users[0].address);
expect(position.amount).to.equal(amount);
expect(position.power).to.be.gt(0);
await time.increase(duration + 1);
const balanceBefore = await raacToken.balanceOf(users[0].address);
await expect(veRAACToken.connect(users[0]).withdraw())
.to.emit(veRAACToken, "Withdrawn")
.withArgs(users[0].address, amount);
const balanceAfter = await raacToken.balanceOf(users[0].address);
expect(balanceAfter - balanceBefore).to.equal(amount);
});
Impact
Overwritten lock amounts will be stuck in the contract and users will be unable to recover tokens when withdrawing.
Tools Used
Manual review
Recommendations
Consider adding a check to prevent users from overwriting their locks
