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

`veRaacToken::lock` does not have functionality to prevenet users from calling the function again which leads to lose of values

nikolaihristov1999

Summary

The veRAACToken::lock function does not prevent a user from creating a new lock when an existing lock already exists. As a result, if a user calls lock more than once, the initial lock is overwritten and its benefits (such as voting power or boost) are lost, leading to potential value loss for the user.

Vulnerability Details

The lock function in the veRAACToken contract is designed to transfer RAAC tokens from the user, create and store a new lock, and mint corresponding veRAAC tokens representing voting power. However, the current implementation does not check whether a user already has an existing lock. This means that if a user calls lock a second time:

  • The previous lock state is overwritten by the new lock.

  • The voting power and boost calculations of the previous lock are lost.

  • The user's previously locked tokens become effectively "lost" or non-recoverable under the intended benefit scheme, as only the new lock will be active.
    The relevant snippet is:

unction 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
raacToken.safeTransferFrom(msg.sender, address(this), amount);
​
// Calculate unlock time
uint256 unlockTime = block.timestamp + duration;
​
// Create lock position
_lockState.createLock(msg.sender, amount, duration);
_updateBoostState(msg.sender, amount);
​
// Calculate initial voting power
(int128 bias, int128 slope) = _votingState.calculateAndUpdatePower(msg.sender, amount, unlockTime);
​
// Update checkpoints
uint256 newPower = uint256(uint128(bias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
​
// Mint veTokens
_mint(msg.sender, newPower);
​
emit LockCreated(msg.sender, amount, unlockTime);
}

There is no check to verify whether a lock already exists for a user. Thus, subsequent calls to lock by the same user will override or replace the prior lock without any safeguards—resulting in a loss of the previous lock’s values and benefits.

Proof of Concept
Append the follow code to the veRaacToken.test.js file

it("calling lock again lead to direct lose of values for user", async () => {
const initalBalanceOfUser = await raacToken.balanceOf(users[0].address);
const amount = ethers.parseEther("500");
const duration = 365 * 24 * 3600; // 1 year
​
// Create lock first
await veRAACToken.connect(users[0]).lock(amount, duration);
​
// Wait for the transaction
// Verify lock position
const position = await veRAACToken.getLockPosition(users[0].address);
expect(position.amount).to.equal(amount);
expect(position.power).to.be.gt(0);
​
// now call again the lock
await veRAACToken.connect(users[0]).lock(amount, duration);
​
// everything remains the same with the difference that we have locked lost our previous lock
const position2 = await veRAACToken.getLockPosition(users[0].address);
expect(position2.amount).to.equal(amount);
expect(position2.power).to.be.gt(0);
​
const balanceAfter = await raacToken.balanceOf(users[0].address);
expect(initalBalanceOfUser - balanceAfter).to.equal(amount * ethers.toBigInt("2"));
});

Impact

  • Loss of Locked Benefits: Users who inadvertently or maliciously call lock multiple times lose the benefits (voting power and boost) from their initial lock.

  • Economic Disadvantage: Overwriting an existing lock could lead to underrepresentation in governance and reduced reward boosts.

  • User Confusion: The absence of a safeguard might lead users to believe that multiple locks can coexist, when in fact any additional call resets their lock state, causing potential loss of funds or misallocation of voting power.

Tools Used

  • Manual Code Review

  • Unit Testing (e.g., the provided test case in veRAACToken.test.js demonstrates the issue)

Recommendations

Prevent users from calling lock if they already possess an active lock. A simple mitigation is to add a check at the beginning of the function to revert if a lock already exists. For example, modify the lock function as follows:

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();
}
+ // Prevent creating a new lock if one already exists for the user.
+ if (_lockState.getLock(msg.sender).exists) {
+ revert LockAlreadyExists();
+ }
​
// 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;
​
// Create lock position
_lockState.createLock(msg.sender, amount, duration);
_updateBoostState(msg.sender, amount);
​
// Calculate initial voting power
(int128 bias, int128 slope) = _votingState.calculateAndUpdatePower(msg.sender, amount, unlockTime);
​
// Update checkpoints
uint256 newPower = uint256(uint128(bias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
​
// Mint veTokens
_mint(msg.sender, newPower);
​
emit LockCreated(msg.sender, amount, unlockTime);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 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.