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

Potential fund lock in StakingPool due to minimum deposits in OperatorVCS and CommunityVCS

iamthesvn

Summary

The StakingPool contract's withdrawal mechanism, specifically in the _withdrawLiquidity function, may lead to user funds becoming inaccessible due to minimum deposit requirements in the OperatorVCS and CommunityVCS strategies. Late withdrawers may find a portion of their funds trapped in the contract, unable to be withdrawn even when theoretically available.

Relevant links

Vulnerability Details

The vulnerability is present in the _withdrawLiquidity function of the StakingPool contract. This function attempts to withdraw funds from the two strategies (OperatorVCS and CommunityVCS) but does not account for their minimum deposit requirements, potentially leaving funds trapped.

function _withdrawLiquidity(uint256 _amount, bytes[] calldata _data) private {
uint256 toWithdraw = _amount;
​
for (uint256 i = strategies.length; i > 0; i--) {
IStrategy strategy = IStrategy(strategies[i - 1]);
uint256 strategyCanWithdrawdraw = strategy.canWithdraw();
​
if (strategyCanWithdrawdraw >= toWithdraw) {
strategy.withdraw(toWithdraw, _data[i - 1]);
break;
} else if (strategyCanWithdrawdraw > 0) {
strategy.withdraw(strategyCanWithdrawdraw, _data[i - 1]);
toWithdraw -= strategyCanWithdrawdraw;
}
}
}

The canWithdraw function in each strategy (OperatorVCS and CommunityVCS) likely returns the amount that can be withdrawn while maintaining the minimum required deposit. As the pool's liquidity decreases due to withdrawals, these strategies will approach their minimum deposit amounts. Once a strategy reaches its minimum, strategyCanWithdrawdraw will return zero, and no further withdrawals will be possible from that strategy.
This design can lead to a situation where funds equal to the sum of minimum deposits across both strategies become permanently locked in the contract, inaccessible to users.

Impact

The impact of this vulnerability is severe, with a moderate to high likelihood of occurrence:

Likelihood: Medium

  1. As users withdraw over time, the likelihood of encountering this issue increases.

  2. The problem becomes more pronounced as the total pool size decreases relative to the sum of minimum deposits required by OperatorVCS and CommunityVCS.

Severity: Medium

  1. Late withdrawers may find a portion of their funds inaccessible.

  2. This could lead to a loss of user trust and potential financial losses for affected users.

  3. In extreme cases, a significant portion of the protocol's total value could become locked.

The combination of increasing likelihood over time and high severity makes this a critical issue to address.

Tools Used

Manual review

Proof of concept

Consider the following scenario:

  1. The StakingPool has 2 strategies: OperatorVCS and CommunityVCS, each with a minimum deposit of 100 tokens.

  2. Initially, the pool has 1000 tokens, distributed as 500 tokens in each strategy.

  3. Users withdraw 750 tokens, leaving 250 tokens in the pool.

  4. The remaining 250 tokens are distributed as 100 in OperatorVCS and 150 in CommunityVCS.

  5. A user attempts to withdraw 100 tokens.

function testWithdrawalLock() public {
// Setup
StakingPool pool = new StakingPool();
MockOperatorVCS operatorVCS = new MockOperatorVCS(100); // Min deposit 100
MockCommunityVCS communityVCS = new MockCommunityVCS(100); // Min deposit 100
pool.addStrategy(address(operatorVCS));
pool.addStrategy(address(communityVCS));
// Initial deposits
pool.deposit(1000);
assert(operatorVCS.balance() == 500);
assert(communityVCS.balance() == 500);
// Withdrawals
pool.withdraw(750);
assert(operatorVCS.balance() == 100);
assert(communityVCS.balance() == 150);
// Attempt to withdraw 100
try pool.withdraw(100) {
assert(false, "Withdrawal should have failed");
} catch Error(string memory reason) {
assert(keccak256(abi.encodePacked(reason)) == keccak256(abi.encodePacked("Insufficient liquidity")));
}
// Check balances haven't changed
assert(operatorVCS.balance() == 100);
assert(communityVCS.balance() == 150);
}

In this scenario, the user cannot withdraw 100 tokens despite there being 250 tokens in the pool, because 200 tokens are locked as minimum deposits across the two strategies.

Recommendations

There doesn't seem to be a trivial fix for this.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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