Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

lock will overwrite locks and brick funds

pyro

Summary

lock will overwrite locks and brick funds

Vulnerability Details

Users make locks with lock, which takes amount of raacToken and transfers it to the contract, then creates a lock

https://github.com/Cyfrin/2025-02-raac/blob/main/contracts/core/tokens/veRAACToken.sol#L212

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();
​
// Do the transfer first - this will revert with ERC20InsufficientBalance if user doesn't have enough tokens
raacToken.safeTransferFrom(msg.sender, address(this), amount);
​
// Calculate unlock time
uint256 unlockTime = block.timestamp + duration;
​
//@audit will overwrite prev. locks
_lockState.createLock(msg.sender, amount, duration);

However the issue is that we overwrite the _lockState for that specific user

https://github.com/Cyfrin/2025-02-raac/blob/main/contracts/libraries/governance/LockManager.sol#L117

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;
//@audit state.locks[user] is in storage meaning that we set it
state.locks[user] = Lock({
amount: amount,
end: end,
exists: true
});
​
state.totalLocked += amount;
​
emit LockCreated(user, amount, end);
return end;
}

Impact

Users who have called lock 2 times will only be able to withdraw the second amount, completely bricking the first one. This is because we have overwritten _lockState.locks[msg.sender].

https://github.com/Cyfrin/2025-02-raac/blob/main/contracts/core/tokens/veRAACToken.sol#L311

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();
​
// get lock
uint256 amount = userLock.amount;
uint256 currentPower = balanceOf(msg.sender);
​
// ...
​
// transfer amount from that lock
raacToken.safeTransfer(msg.sender, amount);

This is not a user error as nothing prevent the user from calling this function again, nor does it say it's dangerous, while at the same time it mints him more voting tokens, giving the impression that works fine.

Tools Used

Manual review

Recommendations

Disallow users from having multiple locks.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

veRAACToken::lock called multiple times, by the same user, leads to loss of funds

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!