Summary
A user can lose funds by repeating lock to veRAACToken.
Vulnerability Details
veRAACToken.sol#lock() function is as follows.
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);
}
As we can see above, there is no check that user(msg.sender) has already created lock.
And LockManager.sol#createLock() function is as follows.
function createLock(
LockState storage state,
address user,
uint256 amount,
uint256 duration
) internal returns (uint256 end) {
if (state.minLockDuration != 0 && state.maxLockDuration != 0) {
if (duration < state.minLockDuration || duration > state.maxLockDuration)
revert InvalidLockDuration();
}
if (amount == 0) revert InvalidLockAmount();
end = block.timestamp + duration;
state.locks[user] = Lock({
amount: amount,
end: end,
exists: true
});
state.totalLocked += amount;
emit LockCreated(user, amount, end);
return end;
}
Here, there isn't any check, too.
So a user can repeat creating lock by overriding previous locking.
Impact
This vulnerability causes two impacts.
The user loses previously locked funds.
His balance increases differently from his power.
This prevents running some functions such as increase().
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);
}
Here, newPower is calculated from newly created lock and balanceOf(msg.sender) contains overrided balance.
Tools Used
Manual review
Recommendations
Modify veRAACToken.sol#lock() function as follows.
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);
}
