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

[M-1] `veRAACToken::lock` function can be called multiple times by the same user leading to loss of funds

xkeywordx

Summary

The veRaacToken contract allows users to lock RAAC tokens to receive voting power and boost capabilities. The system is designed to allow users to open a single lock position that can be adjusted/managed via the increase and extend functions. This is reinforced by the fact that the LockManager::LockState struct contains a mapping mapping(address => Lock) locks; that holds a reference from the user's address to their Lock position that looks like this

struct Lock {
uint256 amount; // Amount of tokens locked
uint256 end; // Timestamp when lock expires
bool exists; // Flag indicating if lock exists
}

Vulnerability Details

The issue is that there are no checks that prevent the same user from calling the veRAACToken::lock function multiple times. If a user calls the lock function multiple times, the amount field stored in the Lock struct will be overridden every time. Although the user gets the right amount of new veRaacToken tokens minted to their address and their voting power is incremented correctly, this will lead to a direct loss of funds because the user is unable to withdraw previously locked amounts.

The veRAACToken::withdraw function burns all the tokens that the user holds, but only transfers back to the user whatever value was stored in the Lock::amount field during the last deposit.

function withdraw() external nonReentrant {
//..
//..
//@audit this is what the user gets back when withdrawing
uint256 amount = userLock.amount;
//@audit this is what gets burned from the user
uint256 currentPower = balanceOf(msg.sender);
​
//..
//..
_burn(msg.sender, currentPower);
raacToken.safeTransfer(msg.sender, amount);
​
emit Withdrawn(msg.sender, amount);
}

Root cause

The current veRAACToken::lock function doesn't check if the user already has an active locked position.

function lock(uint256 amount, uint256 duration) external nonReentrant whenNotPaused {
//..
//..
​
//@audit overrides a user's current lock
_lockState.createLock(msg.sender, amount, duration);
_updateBoostState(msg.sender, amount);
​
//..
//..
}

Impact

Direct loss of funds for users. Users are unable to withdraw their previously locked RAAC tokens. When attempting to withdraw, the withdraw function will burn all the veRAACToken tokens that the user holds, but it will only transfer back the last amount of RAAC tokens that was stored in the contract's state.

PoC

Put this test into the veRAACToken.test.js file and run it.

it.only("prove users override their lock", async () => {
const amount = ethers.parseEther("1000");
const duration = 365 * 24 * 3600; // 1 year
​
// Create lock first
const tx = await veRAACToken.connect(users[0]).lock(amount, duration);
​
// Wait for the transaction
const receipt = await tx.wait();
​
// Find the LockCreated event
const event = receipt.logs.find(
(log) => log.fragment && log.fragment.name === "LockCreated"
);
// Get the actual unlock time from the event
const actualUnlockTime = event.args[2];
​
// Verify lock position
const position = await veRAACToken.getLockPosition(users[0].address);
expect(position.amount).to.equal(amount);
expect(position.end).to.equal(actualUnlockTime);
expect(position.power).to.be.gt(0);
console.log("Initial lock position amount = ", position.amount);
console.log("Initial lock power = ", position.power);
​
// Verify the unlock time is approximately duration seconds from now
const currentTime = await time.latest();
expect(actualUnlockTime).to.be.closeTo(currentTime + duration, 5); // Allow 5 seconds deviation
​
//call the function the second time and prove that it overrides the values
const newAmount = ethers.parseEther("500");
const newDuration = 365 * 24 * 3600; // 1 year
​
// Create lock first
const newTx = await veRAACToken
.connect(users[0])
.lock(newAmount, newDuration);
​
// Wait for the transaction
const newReceipt = await newTx.wait();
​
// Find the LockCreated event
const event2 = newReceipt.logs.find(
(log) => log.fragment && log.fragment.name === "LockCreated"
);
// Get the actual unlock time from the event
const newActualUnlockTime = event2.args[2];
​
//@audit lock position is overridden
const newPosition = await veRAACToken.getLockPosition(users[0].address);
//@audit we overridden the amount and we only have 500 tokens instead of 1500
expect(newPosition.amount).to.equal(newAmount);
expect(newPosition.end).to.equal(newActualUnlockTime);
expect(newPosition.power).to.be.gt(0);
console.log("Position amount after second lock = ", newPosition.amount);
console.log("Position power after second lock = ", newPosition.power);
​
// Verify the unlock time is approximately duration seconds from now
const newCurrentTime = await time.latest();
expect(newActualUnlockTime).to.be.closeTo(
newCurrentTime + newDuration,
5
); // Allow 5 seconds deviation
});

Test output

npm run test:unit:tokens
​
> @raac/core@1.0.0 test:unit:tokens
> npx hardhat test test/unit/core/tokens/*.test.js
​
veRAACToken
Lock Mechanism
Initial lock position amount = 1000000000000000000000n
Initial lock power = 250000000000000000000n
Position amount after second lock = 500000000000000000000n
Position power after second lock = 375000000000000000000n
✔ prove users override their lock (66ms)
​
​
1 passing (2s)

Tools Used

Manual review

Recommended Mitigation

Check if a user already has a lock position created and revert the transaction if the lock already exists. require(!_lockState.locks[msg.sender].exists, "User already has a locked position open");

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month 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.