Description

In ConfiguratorLib, the function finalizeUpdateConfigs is used to update SafetyModule's settings. This function checks safetyModuleState_ == SafetyModuleState.TRIGGERED.

But this check could be bypassed by the owner. The owner of SafetyModule could call pause to transition the state from TRIGGERED to PAUSED. After that, the owner could call finalizeUpdateConfigs to update settings and then return back to TRIGGERED by calling unpause.

The state being SafetyModuleState.TRIGGERED means that the contract is ready to be slashed by using maxSlashPercentage and blocking the user's redeem.

However, in this case, maxSlashPercentage could be changed in applyConfigUpdates, which is called from finalizeUpdateConfigs. The user's deposit could be slashed more than they expect.

  function finalizeUpdateConfigs(
    // ...
  ) internal {
!   if (safetyModuleState_ == SafetyModuleState.TRIGGERED) revert ICommonErrors.InvalidState();
    // ...
!   applyConfigUpdates(reservePools_, triggerData_, delays_, receiptTokenFactory_, configUpdates_);

  function applyConfigUpdates(
    // ...
  ) public {
    // Update existing reserve pool maxSlashPercentages. Reserve pool assets cannot be updated.
    uint8 numExistingReservePools_ = uint8(reservePools_.length);
    for (uint8 i = 0; i < numExistingReservePools_; i++) {
!     reservePools_[i].maxSlashPercentage = configUpdates_.reservePoolConfigs[i].maxSlashPercentage;
    }
    // ...
  }

Impact

The user could check the risk of the pool's maxSlashPercentage before depositing and redeem before the update, because there is a delay for updating.

However, in this case, a malicious owner of SafetyModule could initialize the contract with a low maxSlashPercentage. Then, the owner could update the slash parameter to as high as 100% in TRIGGERED.

As a result of blocking the redeem in TRIGGERED, the user's balance is locked before updating and slashing. This could result in the user's deposit being slashed more than they expected by updating maxSlashPercentage.

Recommendations

We recommend making the update queue zero when pausing the contract. This could prevent SafetyModule from updating in a triggered state.

Remediation