Summary
In the staking system, the StakingPool contract manages user deposits and distributes them across various strategies.
Deposits are allocated to strategies in ascending order (from the first to the last), and withdrawals are processed in descending order (from the last to the first).
This allocation method can lead to an imbalance where the last strategy handles most of the withdrawals, resulting in minimal or zero depositChange for that strategy during reward calculations.
And this imbalance can affect the calculation and distribution of rewards and fees, potentially causing delays in withdrawals, especially during the unbonding period of vault groups.
Vulnerability Details
Deposits are allocated to strategies in ascending order. Deposits are made to strategies starting from the first one in the array. Each strategy receives deposits up to its capacity (strategyCanDeposit).
The remaining deposits are passed to the next strategy.
Contract: StakingPool.sol
472:
473: * @notice Deposits available liquidity into strategies
474: * @dev deposits into strategies in ascending order, only moving to the next once the current is full
475: * @param _data list of deposit data passed to strategies
476: **/
477: function _depositLiquidity(bytes[] calldata _data) private {
478: uint256 toDeposit = token.balanceOf(address(this));
479: if (toDeposit > 0) {
480: for (uint256 i = 0; i < strategies.length; i++) {
481: IStrategy strategy = IStrategy(strategies[i]);
482: uint256 strategyCanDeposit = strategy.canDeposit();
483: if (strategyCanDeposit >= toDeposit) {
484: strategy.deposit(toDeposit, _data[i]);
485: break;
486: } else if (strategyCanDeposit > 0) {
487: strategy.deposit(strategyCanDeposit, _data[i]);
488: toDeposit -= strategyCanDeposit;
489: }
490: }
491: }
492: }
Withdrawals are processed in descending order. Withdrawals are initiated from the last strategy and proceed backward.
Each strategy handles withdrawals up to the amount it can withdraw. Remaining withdrawals are passed to the previous strategy.
Contract: StakingPool.sol
494:
495: * @notice Withdraws liquidity from strategies
496: * @dev withdraws from strategies in descending order only moving to the next once once the current is empty
497: * @param _amount amount to withdraw
498: * @param _data list of withdrawal data passed to strategies
499: **/
500: function _withdrawLiquidity(uint256 _amount, bytes[] calldata _data) private {
501: uint256 toWithdraw = _amount;
502:
503: for (uint256 i = strategies.length; i > 0; i--) {
504: IStrategy strategy = IStrategy(strategies[i - 1]);
505: uint256 strategyCanWithdrawdraw = strategy.canWithdraw();
506:
507: if (strategyCanWithdrawdraw >= toWithdraw) {
508: strategy.withdraw(toWithdraw, _data[i - 1]);
509: break;
510: } else if (strategyCanWithdrawdraw > 0) {
511: strategy.withdraw(strategyCanWithdrawdraw, _data[i - 1]);
512: toWithdraw -= strategyCanWithdrawdraw;
513: }
514: }
515: }
516:
Let´s check the rewarding approach.
The depositChange represents the net change in a strategy's deposits since the last update. It is calculated in the updateDeposits function of each strategy.
Contract: OperatorVCS.sol
159: function updateDeposits(
160: bytes calldata _data
161: )
162: external
163: override
164: onlyStakingPool
165: returns (int256 depositChange, address[] memory receivers, uint256[] memory amounts)
166: {
167: uint256 minRewards = _data.length == 0 ? 0 : abi.decode(_data, (uint256));
168: uint256 newTotalDeposits = totalDeposits;
169: uint256 newTotalPrincipalDeposits;
170: uint256 vaultDeposits;
171: uint256 operatorRewards;
172:
173: uint256 vaultCount = vaults.length;
174: address receiver = address(this);
175: for (uint256 i = 0; i < vaultCount; ++i) {
176: (uint256 deposits, uint256 principal, uint256 rewards) = IOperatorVault(
177: address(vaults[i])
178: ).updateDeposits(minRewards, receiver);
179: vaultDeposits += deposits;
180: newTotalPrincipalDeposits += principal;
181: operatorRewards += rewards;
182: }
183:
184: uint256 balance = token.balanceOf(address(this));
185: depositChange = int256(vaultDeposits + balance) - int256(totalDeposits);
186:
187: if (operatorRewards != 0) {
188: receivers = new address[](1 + (depositChange > 0 ? fees.length : 0));
189: amounts = new uint256[](receivers.length);
190: receivers[0] = address(this);
191: amounts[0] = operatorRewards;
192: unclaimedOperatorRewards += operatorRewards;
193: }
194:
195: if (depositChange > 0) {
196: newTotalDeposits += uint256(depositChange);
197:
198: if (receivers.length == 0) {
199: receivers = new address[](fees.length);
200: amounts = new uint256[](receivers.length);
201:
202: for (uint256 i = 0; i < receivers.length; ++i) {
203: receivers[i] = fees[i].receiver;
204: amounts[i] = (uint256(depositChange) * fees[i].basisPoints) / 10000;
205: }
206: } else {
207: for (uint256 i = 1; i < receivers.length; ++i) {
208: receivers[i] = fees[i - 1].receiver;
209: amounts[i] = (uint256(depositChange) * fees[i - 1].basisPoints) / 10000;
210: }
211: }
212: } else if (depositChange < 0) {
213: newTotalDeposits -= uint256(depositChange * -1);
214: }
215:
216: if (balance != 0) {
217: token.safeTransfer(address(stakingPool), balance);
218: newTotalDeposits -= balance;
219: }
220:
221: totalDeposits = newTotalDeposits;
222: totalPrincipalDeposits = newTotalPrincipalDeposits;
223: }
In a scenario with continuous deposits and withdrawals where the maturity remains but the last strategy handles all these ins/outs;
The first strategies receive the most deposits and have a positive depositChange. (Due to being deposited in ascending order and not being withdrawn afterward)
The last strategy handles most withdrawals, resulting in a negative or zero depositChange. (Due to being withdrawn in descending order and being slack because of the in/outs)
This imbalance affects the calculation of rewards and fees and can be clearly seen in _updateStrategyRewards function that is responsible for the distribution of rewards/fees based on balance changes in the strategies since the last update.
Contract: StakingPool.sol
522: function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
523: int256 totalRewards;
524: uint256 totalFeeAmounts;
525: uint256 totalFeeCount;
526: address[][] memory receivers = new address[][]();
527: uint256[][] memory feeAmounts = new uint256[][]();
528:
529:
530: for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
531: IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
532:
533: (
534: int256 depositChange,
535: address[] memory strategyReceivers,
536: uint256[] memory strategyFeeAmounts
537: ) = strategy.updateDeposits(_data);
538: totalRewards += depositChange;
539:
540: if (strategyReceivers.length != 0) {
541: receivers[i] = strategyReceivers;
542: feeAmounts[i] = strategyFeeAmounts;
543: totalFeeCount += receivers[i].length;
544: for (uint256 j = 0; j < strategyReceivers.length; ++j) {
545: totalFeeAmounts += strategyFeeAmounts[j];
546: }
547: }
548: }
549:
550:
551: if (totalRewards != 0) {
552: totalStaked = uint256(int256(totalStaked) + totalRewards);
553: }
554:
555:
556: if (totalRewards > 0) {
557: receivers[receivers.length - 1] = new address[]();
558: feeAmounts[feeAmounts.length - 1] = new uint256[]();
559: totalFeeCount += fees.length;
560:
561: for (uint256 i = 0; i < fees.length; i++) {
562: receivers[receivers.length - 1][i] = fees[i].receiver;
563: feeAmounts[feeAmounts.length - 1][i] =
564: (uint256(totalRewards) * fees[i].basisPoints) /
565: 10000;
566: totalFeeAmounts += feeAmounts[feeAmounts.length - 1][i];
567: }
568: }
569:
570:
571: if (totalFeeAmounts >= totalStaked) {
572: totalFeeAmounts = 0;
573: }
574:
575:
576: if (totalFeeAmounts > 0) {
577: uint256 sharesToMint = (totalFeeAmounts * totalShares) /
578: (totalStaked - totalFeeAmounts);
579: _mintShares(address(this), sharesToMint);
580:
581: uint256 feesPaidCount;
582: for (uint256 i = 0; i < receivers.length; i++) {
583: for (uint256 j = 0; j < receivers[i].length; j++) {
584: if (feesPaidCount == totalFeeCount - 1) {
585: transferAndCallFrom(
586: address(this),
587: receivers[i][j],
588: balanceOf(address(this)),
589: "0x"
590: );
591: } else {
592: transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
593: feesPaidCount++;
594: }
595: }
596: }
597: }
598:
599: emit UpdateStrategyRewards(msg.sender, totalStaked, totalRewards, totalFeeAmounts);
600: }
As per above, the strategies with positive depositChange contribute to totalRewards (L:538).
So the last strategy, likely having zero or negative depositChange, does not significantly impact totalRewards or generate fees, (Condition at L:556 will be skipped) leading to uneven reward distribution among strategies.
As a result, since the last strategy does not generate fees due to minimal depositChange, fee receivers may receive less than expected.
Besides, vaults are grouped, and each vault group undergoes an unbonding period every week.
The entire unbonding cycle for all groups takes five weeks.
If the last vault group has significantly fewer funds, withdrawals during its unbonding period may be delayed. Users may experience longer wait times for their withdrawals.
Impact
Loss of rewards and fees for some users
Tools Used
Manual Review
Recommendations
We definitely don´t want to recommend a worse situation without going deeper.
We leave this weapon of choice to the Team.
