Summary
stakingPool::_updateRewards allows to transfer all stLink in stakingPool to last strategy fee receiver, including donated , directly transfered, and unusedDeposits stLink in StakingPool.
Vulnerability Details
This occurs because StakingPool contract _updateRewards function wrongly assumes that StakingPool's stlink balance is zero before calculating and minting reward tokens, and when distributing rewards to strategies if the receiver is the last receiver then it sends all of its stLink balance.
function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
uint256 totalFeeCount;
address[][] memory receivers = new address[][]();
for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
if (strategyReceivers.length != 0) {
receivers[i] = strategyReceivers;
feeAmounts[i] = strategyFeeAmounts;
totalFeeCount += receivers[i].length;
for (uint256 j = 0; j < strategyReceivers.length; ++j) {
totalFeeAmounts += strategyFeeAmounts[j];
}
}
}
if (totalFeeAmounts > 0) {
>[1] uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
>[2] _mintShares(address(this), sharesToMint);
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
>[3] if (feesPaidCount == totalFeeCount - 1) {
transferAndCallFrom(
address(this),
receivers[i][j],
>[4] balanceOf(address(this)),
"0x"
);
} else {
transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
}
First the function counts the fee receivers to send to.
[2] mints stLinks based on calculated sharesMint [1]
Next it start to loop receivers and transfer to them stLink
[3] But if its the last receiver then it sends all the stLink pool balance of stakinPool ie address(this), including stLink that wasnt minted in [2].
This means that if StakingPool has a stLink positive balance, previous of minting then its too transfered to the last fee receiver, this happens when stLink is donated , directly transfered or keep in unusedDeposits (when strategies are full) in stakingPool
The following PoC shows the described scenario:
A StakingPool with 2 strategies:
Strategy1, with fee receiver = accounts[0] with 100% fee percentage
Strategy2, with fee receiver = accounts[9] with 100% fee percentage
Staker (accounts[4]) deposits 50 ether of token into StakingPool
Staker receives 50 ether amount of stLink
Staker sends half of his stLink to stakingPool
Now stakingPool has 25 eth of stLink
Link (10 eth) eachis sended to strategies (simulating reward for strategies)
StakingPool::updateStrategyRewards with both strategies address
Now each fee receiver balance should be increased by 10 eth of stLink (calculated, minted and transfered with updateStrategyRewards).
However last strategy receiver of last strategy (accounts[9]) stLink balance increased by 35, ie, the calculated fee for him plus the previous balance existing of stLink in StakingPool.
Add the following auxiliar function and test case in test/core/priorityPool/priority-pool.test.ts
it.only('[R] strategy updateRewards sends more than intended stTokens (all of StakingPool stLink balance) to last fee receiver', async () => {
const { signers, accounts, adrs, token, stakingPool, strategy, sdlPool, pp, withdrawalPool } = await loadFixture(
deployFixture
)
let stakingPool_addr = await stakingPool.getAddress();
console.log("Deploying and adding a new strategy to StakingPool");
const strategy2 = (await deployUpgradeable('StrategyMock', [
adrs.token,
adrs.stakingPool,
toEther(1000),
toEther(100),
])) as StrategyMock;
console.log("[i] Changing strategy owner to accounts[9] (will become strategy fee receiver)");
await strategy2.connect(signers[0]).transferOwnership(accounts[9]);
await stakingPool.addStrategy(strategy2.getAddress());
console.log("[i] Setting strategy fee basis points");
await strategy.setFeeBasisPoints(10000);
await strategy2.setFeeBasisPoints(10000);
const staker = signers[4];
const staker_addr = accounts[4];
const token_amount = toEther(50);
const half_token_amount = token_amount / BigInt(2);
const addr_labels = ["stakingPool", "accounts[0]", "accounts[4]","accounts[9]"]
const addr_array = [stakingPool_addr, accounts[0], accounts[4], accounts[9]]
console.log("\n[i] Initial tokens balance");
await showBalancesFromEther(
token, "token", addr_labels, addr_array
);
await showBalancesFromEther(
stakingPool, "stLink", addr_labels, addr_array
);
console.log(`\n[i] accounts[4] deposits ${token_amount} to StakingPool`)
await pp.connect(staker).deposit(
token_amount, true, ['0x', '0x', '0x']
)
console.log("\n[i] After staking tokens balance");
await showBalancesFromEther(
token, "token", addr_labels, addr_array
);
await showBalancesFromEther(
stakingPool, "stLink", addr_labels, addr_array
);
console.log("\n[i] Staker transfer half of his stLink to SPool");
await stakingPool.connect(staker).transfer(stakingPool_addr,half_token_amount);
console.log("\n[i] Balances after transfer half of staker's stLink to SPool");
await showBalancesFromEther(
token, "token", addr_labels, addr_array
);
await showBalancesFromEther(
stakingPool, "stLink", addr_labels, addr_array
);
console.log("\n[i] Depositing 20 Eth to strategies (simulating rewards) and update rewards (to mint new stLink)")
await token.connect(signers[2]).transfer(strategy, toEther(10))
await token.connect(signers[2]).transfer(strategy2, toEther(10))
console.log("[i] Calling stakingPool::updateRewards");
await stakingPool.updateStrategyRewards([0, 1], '0x')
console.log("[i] Balance after updatingRewards");
await showBalancesFromEther(
token, "token", addr_labels, addr_array
);
await showBalancesFromEther(
stakingPool, "stLink", addr_labels, addr_array
);
console.log("As you can see the last strategy fee receiver get more stLink tokens than expected when StakingPool::updateStrategyRewards is called");
console.log("Strategy2 fee receiver receives the rewards calculated amount, plus all the previous stLink balance of StakingPool");
console.log("But strategy2 fee receiver should have received the same amount as strategy1 fee receiver ie 10 eth of tokens")
})
Impact
Severity: Medium due to this flaw allows to transfer previously stLink balance of StakingPool to last strategy last receiver, but the precondition is that StakingPool has a positive balance of stLink
Tools Used
Manual Review
Recommendations
Disable if branch in StakingPool::_updateStrategyRewards loop and transfer to last strategy last fee receiver to receivers[i][j]:
ie from:
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
if (feesPaidCount == totalFeeCount - 1) {
console.log("\t[iiSP@uSRewards] en if feesPaidCount == totalFeeCount - 1");
console.log("transferAndCallFrom to ",receivers[i][j]);
console.log("\t[iiSP@uSRewards] transferAndCallFrom amount ",balanceOf(address(this)));
transferAndCallFrom(
address(this),
receivers[i][j],
balanceOf(address(this)),
"0x"
);
} else {
console.log("transferAndCallFrom to ",receivers[i][j]);
console.log("\t[iiSP@uSRewards] transferAndCallFrom amount ",feeAmounts[i][j]);
transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
to:
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
