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

Inconsistent `totalStaked` Value After Removing Strategy in StakingPool

0xbrett8571

Summary

The removeStrategy function in the StakingPool contract fails to update the totalStaked variable after withdrawing deposits from the strategy being removed.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L310-L314

IStrategy strategy = IStrategy(strategies[_index]);
uint256 totalStrategyDeposits = strategy.getTotalDeposits();
if (totalStrategyDeposits > 0) {
strategy.withdraw(totalStrategyDeposits, _strategyWithdrawalData);
}

After the strategy.withdraw call, the totalStaked variable should be decreased by the totalStrategyDeposits amount to maintain consistency. However, this update is missing, causing the totalStaked value to remain inflated.

Vulnerability Details

The removeStrategy function allows the owner to remove a strategy from the pool. When a strategy is removed, all the deposits from that strategy are withdrawn using the strategy.withdraw function. However, the totalStaked variable, which represents the total amount of tokens staked in the pool, is not updated to reflect the withdrawal of deposits from the removed strategy.

Impact

The inflated totalStaked value provides an inaccurate representation of the total staked amount in the pool. This can mislead users and other contracts that rely on this value for decision-making or calculations.

function removeStrategy(
uint256 _index,
bytes memory _strategyUpdateData,
bytes calldata _strategyWithdrawalData
) external onlyOwner {
require(_index < strategies.length, "Strategy does not exist");
​
uint256[] memory idxs = new uint256[]();
idxs[0] = _index;
_updateStrategyRewards(idxs, _strategyUpdateData);
​
IStrategy strategy = IStrategy(strategies[_index]);
uint256 totalStrategyDeposits = strategy.getTotalDeposits();
if (totalStrategyDeposits > 0) {
strategy.withdraw(totalStrategyDeposits, _strategyWithdrawalData);
// @audit: totalStaked is not updated after withdrawing deposits
// totalStaked should be decreased by totalStrategyDeposits here
}
​
for (uint256 i = _index; i < strategies.length - 1; i++) {
strategies[i] = strategies[i + 1];
}
strategies.pop();
token.safeApprove(address(strategy), 0);
}

In the code where deposits are withdrawn from the strategy being removed. After the strategy.withdraw call, totalStaked should be decreased by the totalStrategyDeposits amount to maintain consistency with the actual total staked amount in the pool. However, this update is missing, causing totalStaked to remain inflated even after the deposits are withdrawn.

Tools Used

Manual Review

Recommendations

The removeStrategy function should be updated to properly adjust the totalStaked value after withdrawing deposits from the removed strategy. By subtracting the totalStrategyDeposits from totalStaked after the strategy.withdraw call, the totalStaked value will be correctly updated to reflect the withdrawal of deposits from the removed strategy. This ensures that totalStaked remains consistent with the actual total staked amount in the pool.

IStrategy strategy = IStrategy(strategies[_index]);
uint256 totalStrategyDeposits = strategy.getTotalDeposits();
if (totalStrategyDeposits > 0) {
strategy.withdraw(totalStrategyDeposits, _strategyWithdrawalData);
+ totalStaked -= totalStrategyDeposits;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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