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

The lock function in veRAACToken does not check whether the lock exists

biakia

Summary

The lock function in the veRAACToken contract is responsible for creating a new lock position for users who lock their RAAC tokens to receive veRAAC tokens. However, the current implementation does not check whether a user already has an existing lock. This oversight allows users to call the lock function multiple times, overwriting their previous lock while still transferring additional raacToken to the contract. As a result, users may pay more raacToken than the amount actually locked, leading to financial loss and potential fairness issues in the protocol.

Vulnerability Details

The lock function calls createLock function to create lock position:

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;
// Create lock position
_lockState.createLock(msg.sender, amount, duration);
...
...

In createLock function, it does not check whether the lock exists:

function createLock(
LockState storage state,
address user,
uint256 amount,
uint256 duration
) internal returns (uint256 end) {
// Validation logic remains the same
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 a user calls the lock function multiple times, the createLock function overwrites the previous lock record with the new one. However, the raacToken transfer still occurs for each call, meaning the user pays more raacToken than the amount reflected in their final lock.

POC

add the following test case into veRAACToken.test.js

it("the second lock will override the first one", async () => {
const amount = ethers.parseEther("1000");
const duration = 365 * 24 * 3600; // 1 year
// Create lock first
await veRAACToken.connect(users[0]).lock(amount, duration);
await veRAACToken.connect(users[0]).lock(amount, duration);
const balanceOfRaac = await raacToken.balanceOf(await veRAACToken.getAddress());
console.log("raac in veRAACToken:",balanceOfRaac);
// Verify lock position
const expected = ethers.parseEther("2000");
const position = await veRAACToken.getLockPosition(users[0].address);
expect(position.amount).to.equal(expected);
});

run npx hardhat test --grep "the second lock will override the first one"

veRAACToken
Lock Mechanism
raac in veRAACToken: 2000000000000000000000n
1) the second lock will override the first one
​
​
0 passing (15s)
1 failing
​
1) veRAACToken
Lock Mechanism
the second lock will override the first one:
​
AssertionError: expected 1000000000000000000000 to equal 2000000000000000000000.
+ expected - actual
​
-1000000000000000000000
+2000000000000000000000

We can see that 2000000000000000000000 raacToken has been sent to the contract but the lock amout is still 1000000000000000000000.

Impact

Users may lose funds as they pay more raacToken than the amount actually locked. The impact is High, the likelihood is Medium, so the severity is High.

Tools Used

Manual Review

Recommendations

To address this issue, the lock function should include a validation check to ensure that the user does not already have an existing lock. If a lock already exists, the function should revert with an appropriate error message.

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.getLock(msg.sender).exists,"lock already exists");
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!