ravikiran.web3 - VaultCore::depositLv and previewLvDeposit functions will not work as expected due to incorrect implementation of Modifier

[sherlock-admin2]

ravikiran.web3

Medium

VaultCore::depositLv and previewLvDeposit functions will not work as expected due to incorrect implementation of Modifier

Summary

The logic of LVDepositNotPaused modifier condition is incorrectly implemented. In the vault configuration, the pausing of deposits and withdrawals is independently tracked.

   struct VaultConfig {
    // 1 % = 1e18
    uint256 fee;
    uint256 lpBalance;
 ==>    bool isDepositPaused;
 ==>    bool isWithdrawalPaused;
}

The LVDepositNotPaused modifier incorrectly refers to isWithdrawalPaused bool instead of isDepositPaused bool in the below modifier. Due to this, while withdrawals are paused, the depositing functionality is also paused while it is not intended to be paused.

    modifier LVDepositNotPaused(Id id) {
        if (states[id].vault.config.isWithdrawalPaused) {
            revert LVDepositPaused();
        }
        _;
    }

Root Cause

In ModuleState.sol, the logic implementation is incorrect. When the withdrawals are paused, the functions that have the LVDepositNotPaused modifier will also be paused.

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/ModuleState.sol#L108-L113

As the above modifier is attached to depositLv(...) function and previewLvDeposit(...) function, on pausing the withdrawals, both of these functions related to deposit will stop functioning.

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/Vault.sol#L33-L37

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/Vault.sol#L53-L61

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Pause withdrawals by calling the below function with isLVWithdrawalPaused set to true.

     function updatePoolsStatus(
        State storage self,
        bool isPSMDepositPaused,
        bool isPSMWithdrawalPaused,
        bool isLVDepositPaused,
  ===>      bool isLVWithdrawalPaused
    ) internal {
        self.psm.isDepositPaused = isPSMDepositPaused;
        self.psm.isWithdrawalPaused = isPSMWithdrawalPaused;
        self.vault.config.isDepositPaused = isLVDepositPaused;
 ===>       self.vault.config.isWithdrawalPaused = isLVWithdrawalPaused;
    }

2. call depositLv() or previewLvDeposit() and it should revert.

Impact

No response

PoC

No response

Mitigation

Revise the LVDepositNotPaused modifier as below to resolve the issue. The pause should be applicable only if isDepositPaused is set to true for the id.

   modifier LVDepositNotPaused(Id id) {
   -    if (states[id].vault.config.isWithdrawalPaused) {
   +    if (states[id].vault.config.isDepositPaused) {
            revert LVDepositPaused();
        }
        _;
    }

Duplicate of #182