Submission Details
Severity:
Splitter removal will revert due to failed transfer caused by accrued rewards
Summary
Splitters can be removed from the controller by calling LSTRewardsSplitterController::removeSplitter. This function checks if any rewards have been accrued and then splits them among the fee receivers. Afterwards, the remaining balance is sent to the account for which the splitter was deployed. However, instead of transferring the remaining amount, the contract tries to send the balance before the rewards are split:
function removeSplitter(address _account) external onlyOwner {
ILSTRewardsSplitter splitter = splitters[_account];
if (address(splitter) == address(0)) revert SplitterNotFound();
@> uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
@> splitter.withdraw(balance, _account);
}
...
Vulnerability Details
It won’t be possible to remove splitters unless no rewards have been accrued during the last principal deposits update and the LSTRewardsSplitterController::removeSplitter function call. This occurs because the splitter will try to send an amount exceeding its own balance, causing the transaction to revert.
Impact
The following PoC demonstrates the impact of this issue:
See PoC
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.15;
import {Test, console2} from "forge-std/Test.sol";
import {ERC677} from "../src/contracts/core/tokens/base/ERC677.sol";
import {StakingPool} from "../src/contracts/core/StakingPool.sol";
import {StrategyMock} from "../src/contracts/core/test/StrategyMock.sol";
import {WithdrawalPool} from "../src/contracts/core/priorityPool/WithdrawalPool.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {LSTRewardsSplitterController} from "../src/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol";
import {LSTRewardsSplitter} from "../src/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol";
contract WithdrawalPoolTest is Test {
ERC677 token;
StakingPool stakingPool;
StrategyMock strategy;
WithdrawalPool withdrawalPool;
StakingPool stakingPoolImpl = new StakingPool();
WithdrawalPool withdrawalPoolImpl = new WithdrawalPool();
StrategyMock strategyImpl = new StrategyMock();
LSTRewardsSplitterController rewardsSplitterController;
address account0 = makeAddr("account0");
struct StakingPoolParams {
address token;
string name;
string symbol;
StakingPool.Fee[] fee;
uint256 unusedDepositLimit;
}
struct WithdrawalPoolParams {
address token;
address lst;
address pp;
uint256 minWithdrawalAmount;
uint64 minTimeBetweenWithdrawals;
}
function setUp() public {
// Deploy implementation contracts
token = new ERC677("MOCK_TOKEN", "MT", 100_000 ether);
deployStakingPool();
deployStrategy();
deployWithdrawalPool();
uint256 rewardThreshold = 0;
rewardsSplitterController = new LSTRewardsSplitterController(
address(stakingPool),
rewardThreshold
);
}
function testSplitterWithdraw() public {
LSTRewardsSplitter.Fee[]
memory splitterFees = new LSTRewardsSplitter.Fee[](1);
LSTRewardsSplitter.Fee memory splitterFee = LSTRewardsSplitter.Fee({
receiver: address(this),
basisPoints: 100
});
splitterFees[0] = splitterFee;
rewardsSplitterController.addSplitter(address(this), splitterFees);
deal(address(token), address(this), 100 ether);
token.approve(address(stakingPool), type(uint256).max);
bytes[] memory data = new bytes[]();
stakingPool.setPriorityPool(address(this));
stakingPool.addStrategy(address(strategy));
stakingPool.deposit(address(this), 1 ether, data);
// Deposit some tokens into the created splitter
stakingPool.transferAndCall(
address(rewardsSplitterController),
0.1 ether,
""
);
// Mock splitter rewards accrual by donating 1 ether to the staking pool
stakingPool.donateTokens(1 ether);
// panic: arithmetic underflow or overflow
vm.expectRevert();
rewardsSplitterController.removeSplitter(address(this));
}
function deployStakingPool() public {
StakingPool.Fee[] memory _fees = new StakingPool.Fee[]();
StakingPool.Fee memory fee = StakingPool.Fee({
receiver: address(this),
basisPoints: 100
});
_fees[0] = fee;
StakingPoolParams memory params = StakingPoolParams({
token: address(token),
name: "LIQUID_TOKEN",
symbol: "LT",
fee: _fees,
unusedDepositLimit: 1000 ether
});
bytes memory _data = abi.encodeWithSignature(
"initialize(address,string,string,(address,uint256)[],uint256)",
params.token,
params.name,
params.symbol,
params.fee,
params.unusedDepositLimit
);
ERC1967Proxy stakingPoolProxy = new ERC1967Proxy(
address(stakingPoolImpl),
_data
);
stakingPool = StakingPool(address(stakingPoolProxy));
}
function deployStrategy() public {
bytes memory _data;
uint256 _maxDeposits = type(uint256).max;
uint256 _minDeposits = 0;
_data = abi.encodeWithSignature(
"initialize(address,address,uint256,uint256)",
address(token),
address(stakingPool),
_maxDeposits,
_minDeposits
);
ERC1967Proxy strategyProxy = new ERC1967Proxy(
address(strategyImpl),
_data
);
strategy = StrategyMock(address(strategyProxy));
}
function deployWithdrawalPool() public {
WithdrawalPoolParams memory params = WithdrawalPoolParams({
token: address(token),
lst: address(stakingPool),
pp: address(this),
minWithdrawalAmount: 0,
minTimeBetweenWithdrawals: 0
});
bytes memory _data = abi.encodeWithSignature(
"initialize(address,address,address,uint256,uint64)",
params.token,
params.lst,
params.pp,
params.minWithdrawalAmount,
params.minTimeBetweenWithdrawals
);
ERC1967Proxy withdrawalPoolProxy = new ERC1967Proxy(
address(withdrawalPoolImpl),
_data
);
withdrawalPool = WithdrawalPool(address(withdrawalPoolProxy));
}
}
Tools Used
Manual review.
Recommendations
If rewards have been accrued, update the balance to reflect the remaining amount to be transferred:
...
if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
+ balance = IERC20(lst).balanceOf(address(splitter));
splitter.withdraw(balance, _account);
}
...
Updates
Lead Judging Commences
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
In `removeSplitter` the `principalDeposits` should be used as an input for `withdraw` instead of balance after splitting the existing rewards.
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
2,53020 USDC / LOC
42,000 USDC
4,250 USDC
3,750 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.