sorrynotsorry - Deposits can be DoS'ed during the pool cycle in Open state by sandwich attacks

[github-actions[bot]]

sorrynotsorry

high

Deposits can be DoS'ed during the pool cycle in Open state by sandwich attacks

Summary

Deposits can be DoS'ed during the pool cycle in Open state by sandwich attacks

Vulnerability Detail

From the docs; All protection pools operate on a cycle basis. Each pool cycle has a fixed duration and open period (number of days at the beginning of the cycle) during which buyers can actually withdraw their liquidity from the protection pool based on their withdrawal requests. Once a seller requests a withdrawal, s/he can withdraw only after the end of the next pool cycle and during the open duration of the pool cycle.

So when a user deposits liquidity via ProtectionPool::deposit(), below steps are executed;

1. The modifier checks whether the contract is paused and internally calls _deposit
2. _deposit deposits the specified amount of underlying token to the pool and mints the corresponding sToken shares to the receiver. The function first checks whether the current phase of the pool is not OpenToBuyers
3. If passed, on line #1037, it mints _sTokenShares returned from the convertToSToken function. Minted shares are added globally to totalSTokenUnderlying.
4. On Line #1045, it calculates the minimum required capital for the pool to be able to enable protection purchases.
5. If the minimum capital value is passed, it calculates the leverage ratio for the pool on Line #1047
6. If the calculated leverage ratio does not pass the Ceiling Leverage ratio of the pool, the deposit() function successes and events are emitted accordingly.

However, the deposits can be DoS'ed during Step 6.

Calculating Leverage Ratios is below;

1. _deposit internally calls calculateLeverageRatio()
2. calculateLeverageRatio() internally calls _calculateLeverageRatio by passing totalSTokenUnderlying
3. _calculateLeverageRatio returns the leverage ratio scaled to 18 decimals.

   function _calculateLeverageRatio(uint256 _totalCapital)
   internal
   view
   returns (uint256)
   {
   if (totalProtection == 0) {
     return 0;
   }
   return (_totalCapital * Constants.SCALE_18_DECIMALS) / totalProtection;
   }

So after some protections are bought -meaning the total underlying amount of protection bought from the pool by protection buyers is not zero-, an actor/bot can monitor the mempool for deposit() funcsig and sandwich the depositors by a flash loan.

1. A user click deposit() in order to mint X number of shares
2. The attacker takes a very large flash loan and frontruns the user by depositing the loans with deposit() and then backruns with withdraw(). The aim is to mint Y number of shares which is sufficient to touch the leverageRatioCeiling
3. The attacker's shares get minted and the totalSTokenUnderlying gets enormously inflated sufficiently to touch the leverageRatioCeiling
4. The user's deposit reverts with the error ProtectionPoolLeverageRatioTooHigh(_leverageRatio)
5. The attacker withdraws the funds since the pool cycle is in Open status. And it can continue to attack the next depositors as well.

Impact

Denial of the system

Code Snippet

   * @dev Calculate the leverage ratio for the pool.
   * @param _totalCapital the total capital of the pool in underlying token
   * @return the leverage ratio scaled to 18 decimals
   */
  function _calculateLeverageRatio(uint256 _totalCapital)
    internal
    view
    returns (uint256)
  {
    if (totalProtection == 0) {
      return 0;
    }

    return (_totalCapital * Constants.SCALE_18_DECIMALS) / totalProtection;
  } 

Tool used

Manual Review

Recommendation

A withdrawal cooldown period at least as block.number + 1 after the deposit() transaction.

[clems4ev3r]

The request system for withdrawal does that, it is already not possible to deposit and withdraw in the same block, this is not valid