https://github.com/Cyfrin/2025-02-raac/blob/main/contracts/core/governance/boost/BoostController.sol#L177-L203
Summary
The updateUserBoost function in the BoostController contract incorrectly updates the workingSupply of a pool by setting it directly to the new boost amount (newBoost) for a user. This overrides the existing workingSupply of the pool instead of incrementing it, leading to incorrect calculations of the pool's working supply. This issue is particularly problematic when multiple users interact with the pool, as the workingSupply will not reflect the cumulative boost contributions of all users.
Vulnerability Details
Explanation
The updateUserBoost function is designed to update a user's boost amount in a specific pool and adjust the pool's total boost and working supply accordingly. However, the function incorrectly sets the workingSupply to the new boost amount (newBoost) for the user, overriding the existing workingSupply of the pool. This leads to incorrect calculations of the pool's working supply, as it does not account for the contributions of other users.
Root Cause in the Contract Function
The issue lies in the following line of the updateUserBoost function:
function updateUserBoost(address user, address pool) external override nonReentrant whenNotPaused {
if (paused()) revert EmergencyPaused();
if (user == address(0)) revert InvalidPool();
if (!supportedPools[pool]) revert PoolNotSupported();
UserBoost storage userBoost = userBoosts[user][pool];
PoolBoost storage poolBoost = poolBoosts[pool];
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);
}
Instead of incrementing the workingSupply by the difference between the new and old boost amounts, the function sets the workingSupply directly to the new boost amount. This results in the workingSupply being overwritten, leading to incorrect calculations.
Proof of Concept
Scenario Example
User 1 Boosts: User 1 updates their boost, and the workingSupply is set to their boost amount (e.g., 5000), since they have voting power.
User 2 Boosts: User 2 who has no voting power updates their boost, and the workingSupply is overwritten to the minimum boost amount (10000), ignoring User 1's contribution.
Incorrect Working Supply: The workingSupply does not reflect the cumulative boost contributions of all users, leading to incorrect calculations.
Code
The vulnerability is demonstrated in the following Foundry test suite. Convert to foundry project using the steps highlighted here. Then in the test/ folder create a Test file named BoostControllerTest.t.sol and paste the test into it. Make sure the imports path are correct and run the test using forge test --mt testBoostUpdatesBug -vvv :
pragma solidity ^0.8.0;
import "forge-std/Test.sol";
import "contracts/core/governance/boost/BoostController.sol";
import "contracts/mocks/core/pools/MockPool.sol";
import {veRAACToken} from "../../contracts/core/tokens/veRAACToken.sol";
import "contracts/core/tokens/RAACToken.sol";
contract BoostControllerTest is Test {
BoostController public boostController;
veRAACToken public veRAAC;
RAACToken public raacToken;
MockPool public mockPool;
address owner;
address[] public users;
address manager;
bytes32 public constant MANAGER_ROLE = keccak256("MANAGER_ROLE");
uint256 public constant INITIAL_MINT = 1000000 ether;
function setUp() public {
owner = address(this);
users.push(address(0x1));
users.push(address(0x2));
manager = address(0x3);
raacToken = new RAACToken(owner, 100, 50);
raacToken.setMinter(owner);
veRAAC = new veRAACToken(address(raacToken));
boostController = new BoostController(address(veRAAC));
mockPool = new MockPool();
raacToken.manageWhitelist(address(veRAAC), true);
for (uint256 i = 0; i < users.length; i++) {
raacToken.mint(users[i], INITIAL_MINT);
vm.prank(users[i]);
raacToken.approve(address(veRAAC), type(uint256).max);
}
boostController.grantRole(MANAGER_ROLE, manager);
vm.prank(manager);
boostController.modifySupportedPool(address(mockPool), true);
uint256 amount = 1000 ether;
uint256 duration = 365 days;
vm.prank(users[0]);
veRAAC.lock(amount, duration);
}
function testBoostUpdatesBug() public {
address user1 = users[0];
address user2 = users[1];
(uint256 totalBoost, uint256 workingSupply, uint256 baseSupply, uint256 lastUpdateTime) =
boostController.getPoolBoost(address(mockPool));
console.log("Initial Working Supply", workingSupply);
console.log("Initial Total Boost", totalBoost);
vm.prank(user1);
boostController.updateUserBoost(user1, address(mockPool));
(uint256 totalBoost2, uint256 workingSupply2,,) = boostController.getPoolBoost(address(mockPool));
console.log("Working Supply After User1 Boost", workingSupply2);
console.log("Total Boost After User1 Boost", totalBoost2);
vm.prank(user2);
boostController.updateUserBoost(user2, address(mockPool));
(uint256 userBoostAmount,,,) = boostController.getUserBoost(user2, address(mockPool));
(uint256 totalBoost3, uint256 workingSupply3,,) = boostController.getPoolBoost(address(mockPool));
console.log("Final Working Supply After USER2 boost", workingSupply3);
console.log("Final Total Boost After USER2 boost", totalBoost3);
assertEq(workingSupply3, userBoostAmount);
assertGt(workingSupply2, workingSupply3);
assertGt(totalBoost3, workingSupply3);
}
}
In this test:
User 1 updates their boost, and the workingSupply is set to their boost amount.
User 2 updates their boost, and the workingSupply is overwritten to their boost amount, ignoring User 1's contribution.
The final workingSupply does not reflect the cumulative boost contributions of both users.
Bash Result
╰─ forge test --mt testBoostUpdates -vvv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/foundry/BoostControllerTest.t.sol:BoostControllerTest
[PASS] testBoostUpdates() (gas: 252159)
Logs:
Initial Working Supply 0
Initial Total Boost 0
Working Supply After User1 Boost 25000
Total Boost After User1 Boost 25000
Final Working Supply After USER2 boost 10000
Final Total Boost After USER2 boost 35000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.59ms (480.30µs CPU time)
Ran 1 test suite in 10.85ms (1.59ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Tools Used
Recommendations
-
Increment Working Supply:
function updateUserBoost(address user, address pool) external override nonReentrant whenNotPaused {
if (paused()) revert EmergencyPaused(); //@audit-info already using whenNotPaused modifier
if (user == address(0)) revert InvalidPool();
if (!supportedPools[pool]) revert PoolNotSupported();
UserBoost storage userBoost = userBoosts[user][pool];
PoolBoost storage poolBoost = poolBoosts[pool];
uint256 oldBoost = userBoost.amount;
// Calculate new boost based on current veToken balance
uint256 newBoost = _calculateBoost(user, pool, 10000);
userBoost.amount = newBoost;
userBoost.lastUpdateTime = block.timestamp;
// Update pool totals safely
if (newBoost >= oldBoost) {
poolBoost.totalBoost = poolBoost.totalBoost + (newBoost - oldBoost);
+ poolBoost.workingSupply = poolBoost.workingSupply + (newBoost - oldBoost); // Fix: Increment working supply
} else {
poolBoost.totalBoost = poolBoost.totalBoost - (oldBoost - newBoost);
+ poolBoost.workingSupply = poolBoost.workingSupply - (oldBoost - newBoost); // Fix: Decrement working supply
}
- poolBoost.workingSupply = newBoost;
poolBoost.lastUpdateTime = block.timestamp;
emit BoostUpdated(user, pool, newBoost);
emit PoolBoostUpdated(pool, poolBoost.totalBoost, poolBoost.workingSupply);
}
