Liquid Staking

Summary

The StakingPool contract contains a critical vulnerability that allows an attacker to artificially inflate the totalStaked value through the donateTokens function. This inflation can lead to a Denial of Service (DoS) condition for user deposits and significantly manipulate the reward distribution mechanism, potentially reducing rewards to near-zero levels.

Vulnerability Details

The vulnerability stems from the donateTokens function, which allows any user to increase the totalStaked value without minting corresponding liquid staking tokens:

This function lacks access controls and doesn't have any upper limit on the amount that can be donated. An attacker can exploit this to artificially inflate the totalStaked value to an extremely high amount.

The inflated totalStaked value affects several critical functions:

1. The canDeposit function may always return 0, preventing new deposits:

1. The share minting calculation in _updateStrategyRewards is severely impacted:

Impact

1. Denial of Service: Users may be unable to make new deposits for an extended period as the canDeposit function is verified in the depositLiquidity function and also in the getStrategyDepositRoom, which is further used in PriorityPool::checkUpkeep, leading to DoS of critical functions like PriorityPool::performUpkeep.

2. Reward Manipulation: The number of shares minted for fee distribution can be reduced to near-zero levels, severely impacting the reward mechanism and economic incentives of the protocol.

3. Misleading Protocol Metrics: An inflated totalStaked value provides false information about the true state of the pool, potentially misleading users and external systems.

POC

1. If totalStaked is inflated, it will cause the StakingPool::canDeposit function to always return 0. StakingPool::canDeposit is used as a check in the StakingPool::_depositLiquidity function, which may result in incorrect calculations:

   • StakingPool::canDeposit is called in StakingPool::getStrategyDepositRoom, which is used externally in PriorityPool::checkUpkeep, disrupting the functionalities of PriorityPool::performUpkeep.

   • This leads to significant issues in functions like PriorityPool::_deposit and PriorityPool::_depositQueuedTokens that depend on the correct return value of getStrategyDepositRoom.

2. Incorrect calculation and minting of the shares inside StakingPool::_updateStrategyRewards:

Let's assume :

Initial totalStaked = 1,000,000 tokens
Initial totalShares = 1,000,000 shares
totalFeeAmounts = 10,000 tokens

Before the Attack:

After the Attack (with totalStaked inflated to 1e30):

This demonstrates a near 100% reduction in minted shares for rewards.

Tools Used

• Manual Code Review

Recommendations

1. Remove the donateTokens function:

   -- function donateTokens(uint256 _amount) external onlyOwner {

   -- token.safeTransferFrom(msg.sender, address(this), _amount);

   -- totalStaked += _amount;

   -- emit DonateTokens(msg.sender, _amount);

   -- }

2. Use a separate accounting system for the donated tokens. Introduce a new variable to track total deposits separately from totalStaked.

3. Modify the Share Minting Calculation: When calculating shares to mint, exclude the donatedTokens from the totalStaked value to ensure the minted shares reflect the actual staked tokens, not the donated ones.

4. Add rate limiting on the donateTokens function:

   uint256 public constant MAX_DONATION_PER_PERIOD = /* reasonable amount */;

   uint256 public constant DONATION_PERIOD = 1 days;

   mapping(address => uint256) public lastDonationTimestamp;

   mapping(address => uint256) public donationInPeriod;

   ​

   function donateTokens(uint256 _amount) external {

   if (block.timestamp >= lastDonationTimestamp[msg.sender] + DONATION_PERIOD) {

   donationInPeriod[msg.sender] = 0;

   lastDonationTimestamp[msg.sender] = block.timestamp;

   }

   require(donationInPeriod[msg.sender] + _amount <= MAX_DONATION_PER_PERIOD, "Exceeds max donation per period");

   donationInPeriod[msg.sender] += _amount;

   // ... rest of the function

   }