Submission Details
Severity:
Previous lockState is overwritten and tokens lost permanently when user calls lock() again
Description
When lock() is called by a user, it internally calls createLock():
File: contracts/core/tokens/veRAACToken.sol
212: function lock(uint256 amount, uint256 duration) external nonReentrant whenNotPaused {
213: if (amount == 0) revert InvalidAmount();
214: if (amount > MAX_LOCK_AMOUNT) revert AmountExceedsLimit();
215: if (totalSupply() + amount > MAX_TOTAL_SUPPLY) revert TotalSupplyLimitExceeded();
216: if (duration < MIN_LOCK_DURATION || duration > MAX_LOCK_DURATION)
217: revert InvalidLockDuration();
218:
219: // Do the transfer first - this will revert with ERC20InsufficientBalance if user doesn't have enough tokens
220: raacToken.safeTransferFrom(msg.sender, address(this), amount);
221:
222: // Calculate unlock time
223: uint256 unlockTime = block.timestamp + duration;
224:
225: // Create lock position
226:@---> _lockState.createLock(msg.sender, amount, duration);
227: _updateBoostState(msg.sender, amount);
228:
229: // Calculate initial voting power
230: (int128 bias, int128 slope) = _votingState.calculateAndUpdatePower(
231: msg.sender,
232: amount,
233: unlockTime
234: );
235:
236: // Update checkpoints
237: uint256 newPower = uint256(uint128(bias));
238: _checkpointState.writeCheckpoint(msg.sender, newPower);
239:
240: // Mint veTokens
241: _mint(msg.sender, newPower);
242:
243: emit LockCreated(msg.sender, amount, unlockTime);
244: }
If a user calls lock() again, this will overwrite the previous _lockState since there is no check if a lock already exists for the user.
Impact
User permanently loses previously locked tokens and can't withdraw them.
Proof of Concept
Add this inside the describe("Lock Mechanism" section of test/unit/core/tokens/veRAACToken.test.js and run to see it pass:
it("should not allow creating multiple locks for the same user", async function () {
const amount = ethers.parseEther("1000");
const duration = 365 * 24 * 3600; // 1 year
// First lock should succeed
await veRAACToken.connect(users[1]).lock(amount, duration);
// Second lock should succeed too
await veRAACToken.connect(users[1]).lock(amount, duration);
// Verify the lock position has only `amount` instead of `2 * amount`
const position = await veRAACToken.getLockPosition(users[1].address);
expect(position.amount).to.equal(amount); // @audit-issue : passes
});
Mitigation
Add the following check in lock():
require(!_lockState.locks[msg.sender].exists, "lock already exists");
Updates
Lead Judging Commences
inallhonesty 3 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.