Summary
In the veRAACToken::increase function, the calculation of the new voting power of the user uses an amount double that which the user intended to increasehis lock position by.
Vunerability Details
The veRAACToken::increase function is used to increases the amount of locked RAAC tokens whithout changing the unlock time.
File: contracts/core/tokens/veRAACToken.sol#L251-L273
function increase(uint256 amount) external nonReentrant whenNotPaused {
_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,
userLock.amount + amount,
userLock.end
);
uint256 newPower = uint256(uint128(newBias));
_checkpointState.writeCheckpoint(msg.sender, newPower);
raacToken.safeTransferFrom(msg.sender, address(this), amount);
_mint(msg.sender, newPower - balanceOf(msg.sender));
emit LockIncreased(msg.sender, amount);
}
As can be seen in the above code excerpt, the issue is that the amount used to calculates the new voting power is userLock.amount + amount where userLock.amount is supposed to be the old amount of locked RAAC tokens that the user wants to increase. In reality, we already added amount to the old amount of locked tokens when increasing the lock using the LockManager::increaseLock function.
File: contracts/libraries/governance/LockManager.sol#L152-L170
function increaseLock(
LockState storage state,
address user,
uint256 additionalAmount
) internal {
Lock storage lock = state.locks[user];
if (!lock.exists) revert LockNotFound();
if (lock.end <= block.timestamp) revert LockExpired();
if (lock.amount + additionalAmount > state.maxLockAmount) revert AmountExceedsLimit();
@-> lock.amount += additionalAmount;
state.totalLocked += additionalAmount;
emit LockIncreased(user, additionalAmount);
}
This means that userLock.amount + amount in reality is userLock.amount + amount + amount.
Impact
This increases the voting power by more than it should.
Proof Of Concept
Place the following test in test/unit/core/tokens/veRAACToken.test.js.
it.only("should increase voting power by more than it should", async () => {
const initialAmount = ethers.parseEther("1000");
const additionalAmount = ethers.parseEther("500");
const duration = 365 * 24 * 3600;
await veRAACToken.connect(users[0]).lock(initialAmount, duration);
const expectedVotingPower = ((initialAmount + additionalAmount) * BigInt(duration)) / BigInt(1460 * 24 * 3600);
await expect(veRAACToken.connect(users[0]).increase(additionalAmount))
.to.emit(veRAACToken, "LockIncreased")
.withArgs(users[0].address, additionalAmount);
const position = await veRAACToken.getLockPosition(users[0].address);
expect(position.amount).to.equal(initialAmount + additionalAmount);
const votingPower = await veRAACToken.balanceOf(users[0]);
expect(votingPower).to.gt(expectedVotingPower);
});
Run the npx hardhat test test/unit/core/tokens/veRAACToken.test.js command in the terminal.
Impact
Given that, the voting power is used for everything governance voting related, all voting become susceptible to exploitation by anyone who increases their lock position compared to those who didn't increase their lock position.
Tools Used
Manual review.
Recommendations
In veRAACToken::increase make the following change.
function increase(uint256 amount) external nonReentrant whenNotPaused {
// Increase lock using LockManager
_lockState.increaseLock(msg.sender, amount);
_updateBoostState(msg.sender, locks[msg.sender].amount);
// 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));
emit LockIncreased(msg.sender, amount);
}
