Summary
veRaacToken::increase
mints an incorrect amount of veTokens and adds an incorrect amount of votingPower to user.
The fairness of governance votes, gauge reward distribution, and the user's boosted value are impacted.
Vulnerability Details
There are 2 different problems in veRaacToken::increase that lead to:
an incorrect amount of voting power is added to caller.
an incorrect amount of veRaac tokens are minted;
While issue(1) affects the number of new tokens minted, a second issue in the calculation of the number of veTokens to mint aggravate the problem.
Even though these are separate issues, I present them together for easier understanding of the overall impact.
The first problem is that after the amount of tokens are added to the existing lock in _lockState.increaseLock function(tag @1.1 below), the same amount
is added to the already increased userLock.amount
to calculate the newBias
and newSlope
in _votingState.calculateAndUpdatePower (tag @1.2).
The same amount of RaacTokens is added twice when newBias == newPower
is calculated.
The balanceOf(msg.sender)
is then subtracted from the incorrectly calculated newPower.
As a result, both the user's veRaac balance and the voting power are incorrect.
The second problem is that the user's veRaac balance is subtracted from the newPower
to mint the veRaac tokens corresponding to the deposited Raac amount (tag @2).
Instead, the oldPower
should be subtracted from the newPower
.
The voting power and veRaac balance measures different things.
function increase(uint256 amount) external nonReentrant whenNotPaused {
@1.1 _lockState.increaseLock(msg.sender, amount);
_updateBoostState(msg.sender, locks[msg.sender].amount);
LockManager.Lock memory userLock = _lockState.locks[msg.sender];
(int128 newBias, int128 newSlope) = _votingState.calculateAndUpdatePower(
msg.sender,
@1.2 userLock.amount + amount,
userLock.end
);
uint256 newPower = uint256(uint128(newBias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
raacToken.safeTransferFrom(msg.sender, address(this), amount);
@2 _mint(msg.sender, newPower - balanceOf(msg.sender));
emit LockIncreased(msg.sender, amount);
}
The following test explains further these 2 issues and how it should work.
But first edit the getLockEndTime to read the correct storage variable:
function getLockEndTime(address account) external view returns (uint256) {
- return locks[account].end;
+ return _lockState.locks[account].end;
}
Then add the test in the Lock Mechanism from
test/unit/core/tokens/veRAACToken.test.js
Execute it with npx hardhat test --grep "should increase the lock amount and voting power"
.
it("should increase the lock amount and voting power", async () => {
function getLockEndTime(address account) external view returns (uint256) {
return _lockState.locks[account].end;
}
*/
const initialAmount = ethers.parseEther("1000");
const additionalAmount = ethers.parseEther("2000");
const duration = 4 * 365 * 24 * 3600;
const halfDuration = duration / 2;
await raacToken.mint(users[0].address, initialAmount + additionalAmount);
await veRAACToken.connect(users[0]).lock(initialAmount, duration);
const votingPower = await veRAACToken.getVotingPower(users[0].address);
const veBalance = await veRAACToken.balanceOf(users[0].address);
expect (votingPower).to.be.equal(veBalance);
await time.increase(duration / 2);
const currentBlockTimestamp = await time.latest();
const userLockEndTime = await veRAACToken.getLockEndTime(users[0].address);
expect (halfDuration + currentBlockTimestamp).to.be.equal(userLockEndTime);
const votingPower2 = await veRAACToken.getVotingPower(users[0].address);
const veBalance2 = await veRAACToken.balanceOf(users[0].address);
expect (veBalance2).to.be.equal(veBalance);
expect (votingPower2).to.be.closeTo(votingPower / BigInt(2), 10000000);
await veRAACToken.connect(users[0]).increase(additionalAmount);
* 1. the new minted veBalance should be equal to old user veRaac balance
* new veBalance minted = additionalAmount * durationLeft / 4years
* new veBalance minted = 2000 * 2years / 4years = 1000
* 2. the new 'minted' voting power should be equal to the new veBalance minted.
*
* The new 'minted' amounts (veRaac balance and voting power) are added to the old ones:
* user's total veRaac balance = 1000 + 1000 = 2000
* user's total voting power = 1000 / 2 + 1000 = 1500
*/
const votingPower3 = await veRAACToken.getVotingPower(users[0].address);
const veBalance3 = await veRAACToken.balanceOf(users[0].address);
console.log("amounts before `increase`");
console.log("votingPower2: ", votingPower2);
console.log("veBalance2: ", veBalance2);
console.log("amounts after `increase`");
console.log("votingPower3: ", votingPower3);
console.log("veBalance3: ", veBalance3);
});
The test output before fixes from Recommendations
:
amounts before `increase`
votingPower2: 500000000000009248000n
veBalance2: 1000000000000000000000n
amounts after `increase`
votingPower3: 2500000000000000000000n
veBalance3: 2500000000000000000000n
The test output after fixes:
amounts before `increase`
votingPower2: 500000000000009248000n
veBalance2: 1000000000000000000000n
amounts after `increase`
votingPower3: 1500000000000000000000n
veBalance3: 1999999999999990752000n
The veRaac's balanceOf
is used to cast vote for gauges or to calculateBoost for a user's position.
The getVotingPower
(which relies on stored bias) is used in current codebase to calculate pendingRewards, to castVote in governance proposals
Impact
Users receive incorect amounts of veRaac tokens and voting power.
This affects the fairness of governance votes, gauge reward distribution, and the user's boosted value.
Tools Used
Recommendations
Update increase
function:
function increase(uint256 amount) external nonReentrant whenNotPaused {
// Increase lock using LockManager
_lockState.increaseLock(msg.sender, amount);
_updateBoostState(msg.sender, locks[msg.sender].amount);
+ uint256 oldPower = _votingState.getCurrentPower( msg.sender, block.timestamp);
// Update voting power
LockManager.Lock memory userLock = _lockState.locks[msg.sender];
(int128 newBias, int128 newSlope) = _votingState.calculateAndUpdatePower(
msg.sender,
- userLock.amount + amount,
+ userLock.amount,
userLock.end
);
// Update checkpoints
uint256 newPower = uint256(uint128(newBias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
// Transfer additional tokens and mint veTokens
raacToken.safeTransferFrom(msg.sender, address(this), amount);
- _mint(msg.sender, newPower - balanceOf(msg.sender));
+ _mint(msg.sender, newPower - oldPower );
emit LockIncreased(msg.sender, amount);
}