Summary
Contract - veRACC.sol
veRACC.sol::lock() is as follow -
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);
}
LockManager.sol::createLock() ->
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;
}
When lock() function is called by user, a lock is created like -
state.locks[user] = Lock({
amount: amount,
end: end,
exists: true
});
-
Now, suppose same innocent user calls lock() function again (within current lock duration phase), then again LockManager.sol::createLock() will be executed, and state.locks[user] will be updated with new value.
-
For first call, the amount was let's say A1 and for second call A2.
-
which means user's lock state amount has been updated from A1 to A2.
-
Now when lock period is over; user will call veRAAC.sol::withdraw() ->
veRAAC.sol::withdraw() ->
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();
@-1 > 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);
@-2 > raacToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
}
-
it will fetch user's locked amount marked as @-1 > and then transfer raccs to user marked as @-2.
-
Means lock amount A2 will be transferred to user.
-
But remember, user has transfer A1 + A2 amount of tokens into the system but only receiving A2 amounts.
-
which means user has lost A1 amount of raacs.
Vulnerability Details
Same as above.
Impact
Tools Used
Manual
Recommendations
Put a check in lock function, that user can't lock raacs again for existing lock calling lock().
increaseLock() for this purpose, use this.
But even if they mistankly called lock() again for existing lock, make below improvement in LockManager.sol::createLock() ->
function createLock(
LockState storage state,
address user,
uint256 amount,
uint256 duration
) internal returns (uint256 end) {
// Code
.
.
.
state.locks[user] = Lock({
- amount: amount,
+ amount: state.locks[user].amount + amount,
end: end,
exists: true
});
.
.
.
// Code
}
