MorpheusAI

Summary

Distribution.stake function allows stakers to stake in a permanently locked (expired) public pools, which will result in locking their staked assets.

Vulnerability Details

• Users can stake their assets (stEth) in any of the public pools so that they can be rewarded with MOR tokens on the Arbitrum chain based on the amount of their staked assets and how much time these assets have been staked in the contract.

• Users are only allowed to directly stake in public pools, and they can withdraw their staked assets fully or partially after a locking period is passed.

• Stakers can withdraw their staked assets via withdraw function if the withdrawal window is open; which is after some locking time from the latest deposit or when the rewards payout hasn't started:

  require(block.timestamp < pool.payoutStart ||

  (block.timestamp > pool.payoutStart + pool.withdrawLockPeriod &&

  block.timestamp >

  userData.lastStake +

  pool.withdrawLockPeriodAfterStake), "DS: pool withdraw is locked");

• So the user will be able to withdraw if the following a || (b && c) condition returns true, where:

  • a is block.timestamp < pool.payoutStart, and it depends on the pool configuration only.

  • b is block.timestamp > pool.payoutStart + pool.withdrawLockPeriod, and it depends on the pool configuration only.

  • c is userData.lastStake + pool.withdrawLockPeriodAfterStake, and it depends on the pool configuration and the time of the last staking made by the user.

  • a and b conditions are static as they are not affected by the user staking time, while c is dynamic and will change whenever the user stakes.

  • And to get the a || (b && c) check permanently returns false:

    • a should be always false, and this is met when block.timestamp >= pool.payoutStart.

    • b && c check should be always false, and this is met when b is false where block.timestamp <= pool.payoutStart + pool.withdrawLockPeriod

    • false || (false && true) will return false.

• As can be noticed from the above condition; the pool will be permanently locked (expired) if these two conditions are met:
  block.timestamp >= pool.payoutStart and block.timestamp <= pool.payoutStart + pool.withdrawLockPeriod
  where:

  • pool.payoutStart represents the timestamp when the pool starts to pay out rewards.

  • pool.withdrawLockPeriod represents the period in seconds when the user can't withdraw his staked assets.

• So when the pool becomes expired; stakers of this pool will not be able to withdraw their staked assets as the transaction will always revert (DoS'd).

• BUT it was noticed that users can stake in any public pool without checking if this pool is expired or not:

  function stake(uint256 poolId_, uint256 amount_) external poolExists(poolId_) poolPublic(poolId_) {

  _stake(_msgSender(), poolId_, amount_, _getCurrentPoolRate(poolId_));

  }

Impact

Opening the door for users to stake in permanently locked (expired) public pools will result in locking their assets in the Distribution contract, as there is no mechanism to enable them to withdraw their assets after the pool becomes expired.

Proof of Concept

Distribution.stake function

Distribution._stake function

Distribution._withdraw function/L243-L248

Tools Used

Manual Review.

Recommendations

Update _stake function to prevent users from staking in expired pools: