Summary
The delegateBoost and removeBoostDelegation functions in the BoostController contract exhibit an inconsistency in their handling of poolBoost.totalBoost. Specifically, when a user delegates a boost using delegateBoost, the poolBoost.totalBoost is not updated unless updateUserBoost is explicitly called. However, when the delegation is removed using removeBoostDelegation, the poolBoost.totalBoost is decreased regardless of whether it was previously increased. This inconsistency can lead to incorrect accounting of poolBoost.totalBoost, potentially causing underflows or other state inconsistencies.
Vulnerability Details
The delegateBoost function will delegates a boost from the caller to a specified pool (to address) and updates the userBoosts mapping to store the delegation details:
function delegateBoost(
address to,
uint256 amount,
uint256 duration
) external override nonReentrant {
if (paused()) revert EmergencyPaused();
if (to == address(0)) revert InvalidPool();
if (amount == 0) revert InvalidBoostAmount();
if (duration < MIN_DELEGATION_DURATION || duration > MAX_DELEGATION_DURATION)
revert InvalidDelegationDuration();
uint256 userBalance = IERC20(address(veToken)).balanceOf(msg.sender);
if (userBalance < amount) revert InsufficientVeBalance();
UserBoost storage delegation = userBoosts[msg.sender][to];
if (delegation.amount > 0) revert BoostAlreadyDelegated();
delegation.amount = amount;
delegation.expiry = block.timestamp + duration;
delegation.delegatedTo = to;
delegation.lastUpdateTime = block.timestamp;
emit BoostDelegated(msg.sender, to, amount, duration);
}
However, it does not update poolBoost.totalBoost. The poolBoost.totalBoost will be updated in updateUserBoost function:
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);
}
If updateUserBoost is not called after delegateBoost, poolBoost.totalBoost will not reflect the delegated boost. However, removeBoostDelegation will still decrease poolBoost.totalBoost, leading to incorrect accounting:
function removeBoostDelegation(address from) external override nonReentrant {
UserBoost storage delegation = userBoosts[from][msg.sender];
if (delegation.delegatedTo != msg.sender) revert DelegationNotFound();
if (delegation.expiry > block.timestamp) revert InvalidDelegationDuration();
PoolBoost storage poolBoost = poolBoosts[msg.sender];
if (poolBoost.totalBoost >= delegation.amount) {
poolBoost.totalBoost -= delegation.amount;
}
if (poolBoost.workingSupply >= delegation.amount) {
poolBoost.workingSupply -= delegation.amount;
}
poolBoost.lastUpdateTime = block.timestamp;
emit DelegationRemoved(from, msg.sender, delegation.amount);
delete userBoosts[from][msg.sender];
}
POC
add following test case in BoostController.test.js
it("state inconsistencies after removing delegation", async () => {
const poolAddress = user2.address;
await boostController.connect(manager).modifySupportedPool(poolAddress, true);
await veToken.mint(manager.address, ethers.parseEther("1000"));
await boostController.connect(manager).updateUserBoost(manager.address, poolAddress);
const poolBefore = await boostController.getPoolBoost(
poolAddress
);
console.log("Before=====>pool.totalBoost:",poolBefore.totalBoost)
const amount = 13750;
const duration = 7 * 24 * 3600;
await boostController.connect(user1).delegateBoost(poolAddress, amount, duration);
const initialBoost = await boostController.getWorkingBalance(
user1.address,
poolAddress
);
console.log("user1 boost:",initialBoost)
await time.increase(duration);
await expect(
boostController.connect(user2).removeBoostDelegation(user1.address)
).to.emit(boostController, "DelegationRemoved")
.withArgs(user1.address, user2.address, amount);
const poolAfter = await boostController.getPoolBoost(
poolAddress
);
console.log("after =====>pool.totalBoost:",poolAfter.totalBoost)
});
run npx hardhat test --grep "state inconsistencies" , the result is:
BoostController
Delegation System
Before=====>pool.totalBoost: 13750n
user1 boost: 13750n
after =====>pool.totalBoost: 0n
This test case shows that the pool.totalBoost will incorrectly decrease after the delegation is removed.
Impact
The contract's state may become inconsistent, as poolBoost.totalBoost no longer accurately represents the total boost allocated to the pool. The impact is High, the likelihood is Low, so the severity is Medium.
Tools Used
Manual Review
Recommendations
To address this issue, the delegateBoost function should update poolBoost.totalBoost when a boost is delegated. Here is the updated code:
function delegateBoost(
address to,
uint256 amount,
uint256 duration
) external override nonReentrant {
if (paused()) revert EmergencyPaused();
if (to == address(0)) revert InvalidPool();
if (!supportedPools[to]) revert PoolNotSupported();
if (amount == 0) revert InvalidBoostAmount();
if (duration < MIN_DELEGATION_DURATION || duration > MAX_DELEGATION_DURATION)
revert InvalidDelegationDuration();
uint256 userBalance = IERC20(address(veToken)).balanceOf(msg.sender);
if (userBalance < amount) revert InsufficientVeBalance();
UserBoost storage delegation = userBoosts[msg.sender][to];
if (delegation.amount > 0) revert BoostAlreadyDelegated();
uint256 newBoost = _calculateBoost(msg.sender, to, 10000);
PoolBoost storage poolBoost = poolBoosts[to];
poolBoost.totalBoost += newBoost;
poolBoost.workingSupply = newBoost;
poolBoost.lastUpdateTime = block.timestamp;
delegation.amount = newBoost;
delegation.expiry = block.timestamp + duration;
delegation.delegatedTo = to;
delegation.lastUpdateTime = block.timestamp;
emit BoostDelegated(msg.sender, to, amount, duration);
}
