Summary
The working supply of the pool is incorrectly set to the user's newly calculated boost every time the user's boost is updated
Vulnerability Details
In BoostController it is possible to track both user specific boost metrics and pool specific boost metrics. PoolBoost struct is defined as follows:
* @notice Struct to track pool-wide boost metrics
* @param totalBoost The total boost amount for the pool
* @param workingSupply The total working supply including boosts
* @param baseSupply The base supply without boosts
* @param lastUpdateTime The last time pool boosts were updated
*/
struct PoolBoost {
uint256 totalBoost;
uint256 workingSupply;
uint256 baseSupply;
uint256 lastUpdateTime;
}
The totalBoost field of the PoolBoost struct tracks the total number of boosts for a specific pool and is updated every time a user's boost is updated, as it represents the sum of all user boosts. That means, for example, if a user's boost decreases for PoolA, the totalBoost of PoolA will also decrease. The workingSupply field represents the total working supply + all boosts which means it should also get updated with every user boost update. However, as we can see from the function below, the totalWorkingSupply is set to the newly calculated user's boost, which is incorrect. This means that every time a user updates their boost, the value will be overwritten with the boost of that specific user, rather than properly reflecting the total working supply.
* @notice Updates the boost value for a user in a specific pool
* @param user Address of the user whose boost is being updated
* @param pool Address of the pool for which to update the boost
* @dev Calculates new boost based on current veToken balance and updates pool totals
*/
function updateUserBoost(address user, address pool) external override nonReentrant whenNotPaused {
-- SNIPET --
uint256 oldBoost = userBoost.amount;
uint256 newBoost = _calculateBoost(user, pool, 10000);
userBoost.amount = newBoost;
userBoost.lastUpdateTime = block.timestamp;
if (newBoost >= oldBoost) {
poolBoost.totalBoost = poolBoost.totalBoost + (newBoost - oldBoost);
} else {
poolBoost.totalBoost = poolBoost.totalBoost - (oldBoost - newBoost);
}
poolBoost.workingSupply = newBoost;
poolBoost.lastUpdateTime = block.timestamp;
emit BoostUpdated(user, pool, newBoost);
emit PoolBoostUpdated(pool, poolBoost.totalBoost, poolBoost.workingSupply);
}
POC
Add the test to BoostController.test.js
it("should not update pool's working supply with user's boost", async () => {
let [totalBoost, workingSupply, ,] = await boostController.connect(user1).getPoolBoost(mockPool.getAddress());
expect(totalBoost).to.be.eq("0");
expect(workingSupply).to.be.eq("0");
await veToken.mint(user1.address, ethers.parseEther("1000"));
let [, boostedAmount] = await boostController.calculateBoost(
user1.address,
mockPool.getAddress(),
10000
);
let tx = await boostController.connect(user1).updateUserBoost(user1.address, mockPool.getAddress());
let receipt = await tx.wait();
const boostUpdatedEvent = receipt.logs[0];
expect(boostUpdatedEvent.args[0]).to.equal(user1.address);
expect(boostUpdatedEvent.args[1]).to.equal(await mockPool.getAddress());
expect(boostUpdatedEvent.args[2]).to.equal(boostedAmount);
[totalBoost, workingSupply] = await boostController.connect(user1).getPoolBoost(mockPool.getAddress());
expect(totalBoost).to.be.eq(boostedAmount);
expect(workingSupply).to.be.eq(boostedAmount);
await veToken.mint(user2.address, ethers.parseEther("500"));
let [, boostedAmount2] = await boostController.calculateBoost(
user2.address,
mockPool.getAddress(),
10000
);
tx = await boostController.connect(user2).updateUserBoost(user2.address, mockPool.getAddress());
receipt = await tx.wait();
const boostUpdatedEvent2 = receipt.logs[0];
expect(boostUpdatedEvent2.args[0]).to.equal(user2.address);
expect(boostUpdatedEvent2.args[1]).to.equal(await mockPool.getAddress());
expect(boostUpdatedEvent2.args[2]).to.equal(boostedAmount2);
[totalBoost, workingSupply] = await boostController.connect(user1).getPoolBoost(mockPool.getAddress());
expect(totalBoost).to.be.eq(boostedAmount + boostedAmount2);
expect(workingSupply).to.be.eq(boostedAmount2);
});
Impact
Medium
Tools Used
Manual Review
Recommendations
The description of the workingSupply in the PoolBoost struct suggests that it should most likely include baseSupply + totalBoosts. Therefore, assuming that workingSupply has already been properly initialized, when updating a user's specific boost, it should be calculated as follows:
function updateUserBoost(address user, address pool) external override nonReentrant whenNotPaused {
-- SNIPET --
uint256 oldBoost = userBoost.amount;
uint256 newBoost = _calculateBoost(user, pool, 10000);
userBoost.amount = newBoost;
userBoost.lastUpdateTime = block.timestamp;
uint256 oldTotalBoost = poolBoost.totalBoost;
uint256 newTotalBoost;
if (newBoost >= oldBoost) {
newTotalBoost = poolBoost.totalBoost + (newBoost - oldBoost);
} else {
newTotalBoost = poolBoost.totalBoost - (oldBoost - newBoost);
}
poolBoost.totalBoost = newTotalBoost;
if (newTotalBoost > oldTotalBoost) {
poolBoost.workingSupply = poolBoost.workingSupply + (newTotalBoost - oldTotalBoost);
} else {
poolBoost.workingSupply = poolBoost.workingSupply - (oldTotalBoost - newTotalBoost);
}
poolBoost.lastUpdateTime = block.timestamp;
-- SNIPET --
}
