Summary
Missing validation in lock function will cause a loss of locked funds and functionality disruption for users as they can create multiple locks which overrides their existing lock position.
Root Cause
In veRAACToken.sol and LockManager.sol there are missing checks to prevent users from creating multiple locks.
When calling lock(), there is no validation to ensure the user doesn't already have an existing lock position. Similarly, in LockManager.sol#createLock(), no checks exist to prevent overriding existing locks.
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);
}
Impact
The vulnerability causes two main issues:
Users lose their previously locked funds when creating a new lock that overrides their existing position
Users' balance becomes inconsistent with their voting power, preventing usage of functions like increase()
For example, in the increase() function:
function increase(uint256 amount) external nonReentrant whenNotPaused {
_lockState.increaseLock(msg.sender, amount);
_updateBoostState(msg.sender, locks[msg.sender].amount);
LockManager.Lock memory userLock = _lockState.locks[msg.sender];
(int128 newBias, int128 newSlope) = _votingState.calculateAndUpdatePower(
msg.sender,
userLock.amount + amount,
userLock.end
);
uint256 newPower = uint256(uint128(newBias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
raacToken.safeTransferFrom(msg.sender, address(this), amount);
-> _mint(msg.sender, newPower - balanceOf(msg.sender));
emit LockIncreased(msg.sender, amount);
}
The calculation newPower - balanceOf(msg.sender) becomes incorrect due to the overridden balance state.
Mitigation
Add validation in veRAACToken.sol#lock() to prevent creating multiple locks:
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();
+ require(!_lockState.locks[msg.sender].exists, "Already exists");
.........................
emit LockCreated(msg.sender, amount, unlockTime);
}
