Submission Details
Severity:
Users can lose RAACToken if creating lock more than once
Summary
Contract veRAACToken allows RAACToken holders to lock their RAACToken and mint veRAACToken representing their voting power. However, the function veRAACToken::lock() is flaw such that it does not prevent users from locking tokens when users have already locked tokens
Vulnerability Details
The function veRAACToken::lock() collects RAACToken from caller and then create lock for the caller by _lockState.createLock(msg.sender, amount, duration). It is obvious that neither the function veRAACToken::lock() checks if the caller has already created a lock position nor the function LockManager::createLock() checks. This can cause users can call lock() more than once, with the later lock amount overwrite the previous lock amount. This can cause loss of funds because in withdraw flow, the users can only withdraw the later amount.
// veRAACToken
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);
_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);
}
/// LockManager
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, // this is overwrite the lock amount
end: end,
exists: true
});
state.totalLocked += amount;
emit LockCreated(user, amount, end);
return end;
}
PoC
Add the test below to file test/unit/core/tokens/veRAACToken.test.js
describe("Lock Mechanism", () => {
/// ...
it.only("Users can lose funds if creating lock more than once", async () => {
// @audit PoC create lock more than once
const amount = ethers.parseEther("1000");
const duration = 365 * 24 * 3600; // 1 year
// Create lock first
await veRAACToken.connect(users[0]).lock(amount, duration);
let amount2 = ethers.parseEther("10");
// continue to create lock
await veRAACToken.connect(users[0]).lock(amount2, duration);
const position = await veRAACToken.getLockPosition(users[0].address);
expect(position.amount).to.equal(amount2);
// balance before withdraw
let userRAACBalanceBefore = await raacToken.balanceOf(users[0])
await ethers.provider.send("evm_increaseTime", [duration + 1]);
await ethers.provider.send("evm_mine");
await veRAACToken.connect(users[0]).withdraw();
// balance after withdraw lock
let userRAACBalanceAfter = await raacToken.balanceOf(users[0])
let withdrawn = userRAACBalanceAfter - userRAACBalanceBefore
expect(withdrawn).to.eq(amount + amount2)
});
Run the test and console shows:
veRAACToken
Lock Mechanism
1) Users can lose funds if creating lock more than once
0 passing (2s)
1 failing
1) veRAACToken
Lock Mechanism
Users can lose funds if creating lock more than once:
AssertionError: expected 10000000000000000000 to equal 1010000000000000000000.
+ expected - actual
-10000000000000000000
+1010000000000000000000
Impact
Users can lose funds
Tools Used
Manual
Recommendations
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.locks[msg.sender].exists, "already locked");
// 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 5 months ago
Submission Judgement Published Assigned finding tags:
veRAACToken::lock called multiple times, by the same user, leads to loss of funds
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420 USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.