Summary
Strategy has unbounded fees percentage amount on strategy::setFeeBasisPoints, allowing to mint (as stLink) all extra deposits amounts (suchs as rewards) to strategy's fee receiver
Vulnerability Details
StakingPool has a restriction for fees: the sum of all fees must not be greather than 40%.
In StakingPool::initialize:
function initialize(
address _token,
Fee[] memory _fees,
uint256 _unusedDepositLimit
) public initializer {
=> require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%");
And when adding a new Fee:
function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
fees.push(Fee(_receiver, _feeBasisPoints));
=> require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%");
}
This is to ensure that fee receiver wont obtain all rewards distributed for themselfs.
However this doesnt happen for strategy's fee receiver cause strategys allows to set 100% of rewards distributed to its fee receivers using:
function setFeeBasisPoints(uint256 _feeBasisPoints) external {
feeBasisPoints = _feeBasisPoints;
}
So this allows to strategy fee receivers (originally strategy owners, and depends on strategy implementation) to receive 100% of rewards in form of staking tokens, minted when StakingPool::_updateStrategyRewards is called:
function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
if (strategyReceivers.length != 0) {
receivers[i] = strategyReceivers;
}
if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
@=> _mintShares(address(this), sharesToMint);
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
if (feesPaidCount == totalFeeCount - 1) {
@=> transferAndCallFrom(
address(this),
receivers[i][j],
balanceOf(address(this)),
"0x"
);
So, a strategy fee receiver can receive all stLink, leaving strategy token depositor to receive 0 rewards.
The following PoC shows the described scenario:
StakingPool defines single strategy.
An user (account[4]) deposits tokens to StakingPool (via priorityPool) and became deposited for strategy (strategy has room available to deposit)
Rewards are distributed for strategy
Strategy owner changes feeBasisPoints for strategy to 100% to fee receiver
StakingPool::_updateStrategyRewards is called
Strategy owner receives 100% of rewards for himself, depositor (accounts[4]) receives nothing for staking into StakingPool
Append the following auxiliar function and test case in test/core/priorityPool/priority-pool.test.ts:
async function showBalancesFromEther(token, token_name, addr_labels, addrs){
for(var i=0; i < addr_labels.length; i++){
console.log(
`${token_name} balanceOf ${addr_labels[i]}: `,
fromEther(await token.balanceOf(addrs[i]))
);
}
}
it('[R] strategy Fee receiver can receive 100% of rewards', async () => {
const { signers, accounts, adrs, token, stakingPool, strategy, sdlPool, pp, withdrawalPool } = await loadFixture(
deployFixture
)
const staker = signers[4];
const staker_addr = accounts[4];
const token_amount = toEther(10);
console.log("[i] Starting balances");
await showBalancesFromEther(
token,"link",
["accounts[0]","accounts[4]"], [ accounts[0], accounts[4]]
)
await showBalancesFromEther(
stakingPool,"stlink",
["accounts[0]","accounts[4]"], [ accounts[0], accounts[4]]
)
console.log(`\n[i] accounts[4] deposits ${token_amount}`)
await pp.connect(staker).deposit(
token_amount,
true,
['0x', '0x', '0x']
)
console.log("\n[i] After staking balances");
await showBalancesFromEther(
token,"link",
["accounts[0]","accounts[4]"], [ accounts[0], accounts[4]]
)
await showBalancesFromEther(
stakingPool,"stlink",
["accounts[0]","accounts[4]"], [ accounts[0], accounts[4]]
)
console.log("\n[i] Strategy owner changes strategy fee basis points to 10_000")
await strategy.setFeeBasisPoints(10_000)
console.log("[i] Simulating strategy rewards balance change (+20 eth)")
await token.connect(signers[3]).transfer(strategy, toEther(20))
console.log("[i] Updating strategy rewards");
await stakingPool.updateStrategyRewards([0], '0x')
console.log("\n[i] Final balances");
await showBalancesFromEther(
token,"link",
["accounts[0]","accounts[4]"], [ accounts[0], accounts[4]]
)
await showBalancesFromEther(
stakingPool,"stlink",
["accounts[0]","accounts[4]"], [ accounts[0], accounts[4]]
)
})
Observe strategy fee receiver gets all rewards and staker receive nothing for staking into StakingPool
Impact
Severity: High due to this lack of check of strategy fee basis points allows to strategy fee receivers to mint all rewards for himself, leaving StakingPool stakers to receive nothing for stake
Tools Used
Manual Review
Recommendations
StakingPool::addStrategy should check that sum of strategy's fee receivers should not be greater than certain reward for example, the same 40% limit applied to StakingPool sum of fees:
function addStrategy(address _strategy) external onlyOwner {
require(!_strategyExists(_strategy), "Strategy already exists");
require(_strategy.feeBasisPoints() <= 4_000, "Strategy fee to high");
token.safeApprove(_strategy, type(uint256).max);
strategies.push(_strategy);
}
This feeBasisPoints method should be added to IStrategy inteface
