Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Fees can be lost in case of turbulent market conditions or whale attack

Dup1337

Summary

In turbulent market conditions or in case of whale attack, fees accrued by strategies might be bigger than StakingPool#totalStaked storage variable. In this case, fees are completely skipped, making fees funds sent fro strategy locked in the contract.

Vulnerability Details

When strategy rewards are being processed, strategy returns the amount of fees it accrued. It is used then to calculate how much fees to send. However, there is an edge case when totalFeeAmounts are bigger than totalStaked. In this case the code skips fees:

StakingPool.sol
​
function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
// [...]
address[][] memory receivers = new address[][]();
uint256[][] memory feeAmounts = new uint256[][]();
​
// sum up rewards and fees across strategies
for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
​
(
int256 depositChange,
address[] memory strategyReceivers,
uint256[] memory strategyFeeAmounts
@> ) = strategy.updateDeposits(_data);
@> totalRewards += depositChange;
// [...]
// calulate fees if net positive rewards were earned
if (totalRewards > 0) {
receivers[receivers.length - 1] = new address[]();
feeAmounts[feeAmounts.length - 1] = new uint256[]();
totalFeeCount += fees.length;
​
for (uint256 i = 0; i < fees.length; i++) {
receivers[receivers.length - 1][i] = fees[i].receiver;
feeAmounts[feeAmounts.length - 1][i] =
(uint256(totalRewards) * fees[i].basisPoints) /
10000;
totalFeeAmounts += feeAmounts[feeAmounts.length - 1][i];
}
}
​
// safety check
if (totalFeeAmounts >= totalStaked) {
totalFeeAmounts = 0;
}
​
// distribute fees to receivers if there are any
@> if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
_mintShares(address(this), sharesToMint);
// distribute fees to receivers if there are any
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"
);
} else {
@> transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
}
​

So incase there would be a large withdrawal by a single entity or organically by honest users due to turbulent market conditions, the totalStaked variable will diminish badly and remain less than the totalFeeAmounts.

However, the problem is that the strategy sends the funds either way

function updateDeposits(
bytes calldata _data
)
external
override
onlyStakingPool
returns (int256 depositChange, address[] memory receivers, uint256[] memory amounts)
{
// [...]
if (balance != 0) {
@> token.safeTransfer(address(stakingPool), balance);
newTotalDeposits -= balance;
}

So, the fees are locked in the contract in this case. Technically, they can be treated as "unused deposit" that anyone can put into strategies, because of the logic in deposit that allows to put tokens that StakingPool is having into strategies.

If this won't happen, the fees can be taken by the last fee receiver on the next call:

if (feesPaidCount == totalFeeCount - 1) {
transferAndCallFrom(
address(this),
receivers[i][j],
@> balanceOf(address(this)), // @audit recipient will get whole balance
"0x"
);

This might actually have broader consequences, because last fee receiver will get whatever balance of the tokens that the protocol has.

Impact

Loss of rewards

Tools Used

Manual review

Recommendations

Consider queueing fees for later in case that it's bigger than StakingPool#totalStaked.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.