Summary
The veRAACToken::lock
function overwrites the previous lock. This can lead to funds loss for the user.
Vulnerability Details
The veRAACToken::lock
function does not check if the user already has a lock. If user calls the lock
function multiple times, the previous lock will be overwritten. And funds remain unwithdrawable.
/contracts/core/tokens/veRAACToken.sol
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();
raacToken.safeTransferFrom(msg.sender, address(this), amount);
uint256 unlockTime = block.timestamp + duration;
# here previous lock is overwritten
@> _lockState.createLock(msg.sender, amount, duration);
_updateBoostState(msg.sender, amount);
(int128 bias, int128 slope) = _votingState.calculateAndUpdatePower(
msg.sender,
amount,
unlockTime
);
uint256 newPower = uint256(uint128(bias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
_mint(msg.sender, newPower);
emit LockCreated(msg.sender, amount, unlockTime);
}
PoC (foundry)
pragma solidity 0.8.28;
import {Test, console} from "forge-std/Test.sol";
import {RAACToken} from "src/core/tokens/RAACToken.sol";
import {veRAACToken} from "src/core/tokens/veRAACToken.sol";
import {IveRAACToken} from "src/interfaces/core/tokens/IveRAACToken.sol";
contract TestVeRAACTokenLock is Test {
RAACToken public raacToken;
veRAACToken public veToken;
address user;
function setUp() public {
raacToken = new RAACToken(address(this), 0, 0);
raacToken.setMinter(address(this));
veToken = new veRAACToken(address(raacToken));
raacToken.manageWhitelist(address(veToken), true);
user = makeAddr("user");
raacToken.mint(user, 3000);
vm.prank(user);
raacToken.approve(address(veToken), 3000);
}
function test_lock() public {
uint256 userInitialRAACBalance = raacToken.balanceOf(user);
vm.startPrank(user);
veToken.lock(2000, 365 days);
IveRAACToken.LockPosition memory lock = veToken.getLockPosition(user);
console.log("Lock position amount: ", lock.amount);
veToken.lock(1000, 365 days);
IveRAACToken.LockPosition memory lock2 = veToken.getLockPosition(user);
console.log("Lock position amount: ", lock2.amount);
skip(366 days);
veToken.withdraw();
uint256 userFinalRAACBalance = raacToken.balanceOf(user);
vm.stopPrank();
assertEq(userInitialRAACBalance, userFinalRAACBalance, "User should have the same balance");
}
}
Impact
User can lose funds if the lock
function is called multiple times.
Recommendations
Add check if the user already has a lock. If so, revert the transaction. Let user use the increase
function to add more funds to the existing lock.
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();
+ LockManager.Lock memory userLock = _lockState.locks[msg.sender];
+ if (userLock.amount > 0) revert LockAlreadyExist();
// 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);
}