Summary
Users can be DoSed locking their funds using the veRAACToken::lock function due to an incorrect check of totalSupply limit.
Vunerability Details
When locking funds using the veRAACToken::lock function, there is a check on the limit of the totalSupply of veRAACToken to prevent voting power to be more than the desired value. But this check is incorrectly done since it uses the amount of RAAC tokens to be locked instead of the amount of the amount veRAACToken (power) to mint in exchange of RAAC tokens.
File: contracts/core/tokens/veRAACToken.sol#L212-L244
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;
_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);
}
The newPower minted is deducted from bias which is calculated using the VotingPowerLIb::calculateAndUpdatePower function
File: contracts/libraries/governance/VotingPowerLib.sol#L75-L107
function calculateAndUpdatePower(
VotingPowerState storage state,
address user,
uint256 amount,
uint256 unlockTime
) internal returns (int128 bias, int128 slope) {
if (amount == 0 || unlockTime <= block.timestamp) revert InvalidPowerParameters();
uint256 MAX_LOCK_DURATION = 1460 days;
uint256 duration = unlockTime - block.timestamp;
uint256 initialPower = (amount * duration) / MAX_LOCK_DURATION;
bias = int128(int256(initialPower));
slope = int128(int256(initialPower / duration));
uint256 oldPower = getCurrentPower(state, user, block.timestamp);
state.points[user] = RAACVoting.Point({
bias: bias,
slope: slope,
timestamp: block.timestamp
});
_updateSlopeChanges(state, unlockTime, 0, slope);
emit VotingPowerUpdated(user, oldPower, uint256(uint128(bias)));
return (bias, slope);
}
As one can see, the calculated power will always be lower than the locked amount.
Impact
As a result, users might be prevented from locking funds if the amount to lock is > MAX_TOTAL_SUPPLY - totalSupply() even if it won't break any invariant of the contract.
Also, MAX_TOTAL_SUPPLY will never be reached since it is the amount to be lock that is used to perform the verification and the calculated power will always be lower than the locked amount.
Proof of Concept
The following PoC proves the above statement.
it.only("should user's balance difference be lower than lock amount", async () => {
const amount = ethers.parseEther("1000");
await raacToken.mint(users[0].address, amount);
const duration = 365 * 24 * 3600;
const userBalanceBefore = await veRAACToken.balanceOf(users[0]);
await veRAACToken.connect(users[0]).lock(amount, duration);
const userBalanceAfter = await veRAACToken.balanceOf(users[0]);
expect(userBalanceAfter - userBalanceBefore).to.be.lt(amount);
});
Copy the above code in test/unit/core/tokens/veRAACToken.test.js and run npx hardhat test test/unit/core/tokens/veRAACToken.test.js in the terminal.
Tools Used
Manual review.
Recommendations
In veRAACToken::lock(), verify the totalSupply limit with newPower instead of amount.
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));
+ if (totalSupply() + newPower > MAX_TOTAL_SUPPLY) revert TotalSupplyLimitExceeded();
_checkpointState.writeCheckpoint(msg.sender, newPower);
// Mint veTokens
_mint(msg.sender, newPower);
emit LockCreated(msg.sender, amount, unlockTime);
}
