hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

Users can withdraw tokens when the protocol is paused due to inconsistant updates of states. #58

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x9ac6daec0791cbe9f41caaa26a2c32ab03564e7db548aa9a6c118ace35e04a46 Severity: high

Description: Description\ While withdrawing tokens there is a check in _beforeWithdrawCheck() function to see if the protocol is emergency paused or not , however it doesn't check for the normal pause condition which can be problematic and users can still be able to withdraw their tokens while the protocol is paused and not emergency paused.

Attack Scenario\ Let's take a look at _beforeWithdrawCheck function first :

 function _beforeWithdrawCheck(address owner, IPortfolio portfolio, uint256 _tokenAmount) internal view {
        if (protocolConfig().isProtocolEmergencyPaused()) {
            revert ErrorLibrary.ProtocolIsPaused();
        } //@audit doens't check for normal pause condition
        uint256 balanceOfUser = portfolio.balanceOf(owner);
        if (_tokenAmount > balanceOfUser) {
            revert ErrorLibrary.CallerNotHavingGivenPortfolioTokenAmount();
        }
        uint256 balanceAfterRedemption = balanceOfUser - _tokenAmount;
        if (balanceAfterRedemption != 0 && balanceAfterRedemption < protocolConfig().minPortfolioTokenHoldingAmount()) {
            revert ErrorLibrary.CallerNeedToMaintainMinTokenAmount();
        }
    }

The above check while withdrawing is to see if the protocol is emergencyPaused or not and doesn't check anything for normal pause condition.

Now this can be problematic when protocol state reaches a condition like this :

Protocol State: isProtocolPaused = true isProtocolEmergencyPaused = false

In that case the check will pass and user will be able to withdraw even when the protocol is paused and not emergency paused since the check is only for emergencyPaused situation.

Attachments

  1. Proof of Concept (PoC) File

Assume: The protocol owner needs to perform maintenance and wants to ensure no withdrawals can occur during this period.

  1. Calls setEmergencyPause(true, false) to emergency pause the protocol.
  2. Immediately calls setEmergencyPause(false, false) to unpause the emergency state but keep the normal pause active.

Protocol State: isProtocolPaused = true isProtocolEmergencyPaused = false

A user tries to withdraw funds. The _beforeWithdrawCheck function checks isProtocolEmergencyPaused, which is false, and allows the withdrawal to proceed despite the protocol being intended to be paused.

Revised Code File (Optional)

Modify _beforeWithdrawCheck to check both isProtocolPaused and isProtocolEmergencyPaused:

if (protocolConfig().isProtocolPaused() || protocolConfig().isProtocolEmergencyPaused()) {
    revert ErrorLibrary.ProtocolIsPaused();
}

This ensures that if either pause state is active, the withdrawal will be blocked.

langnavina97 commented 1 week ago

This behavior is intentional. When the protocol is paused, users should be able to withdraw their funds. However, when the protocol is in emergency pause, withdrawals are also paused.