Skip to content

Latest commit

 

History

History
105 lines (71 loc) · 3.72 KB

File metadata and controls

105 lines (71 loc) · 3.72 KB

Low Sangria Cricket

High

Usd0PP::emergencyWithdraw() will not work if the contract is paused

Summary

The Usd0PP::emergencyWithdraw() function, designed as an emergency mechanism for withdrawing funds (e.g., in cases of protocol exploitation), fails if the contract is already paused. This restricts the function’s usability during emergencies when the contract might be paused preemptively.

In the current implementation, Usd0PP::emergencyWithdraw() calls the PausableUpgradeable::_pause() function, which reverts if the contract is already paused:

    function emergencyWithdraw(address safeAccount) external {
        Usd0PPStorageV0 storage $ = _usd0ppStorageV0();

        if (!$.registryAccess.hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) {
            revert NotAuthorized();
        }
        IERC20 usd0 = $.usd0;

        uint256 balance = usd0.balanceOf(address(this));
        // get the collateral token for the bond
        usd0.safeTransfer(safeAccount, balance);

        // Pause the contract
@1>     _pause();

        emit EmergencyWithdraw(safeAccount, balance);
    }
    /**
     * @dev Triggers stopped state.
     *
     * Requirements:
     *
     * - The contract must not be paused.
     */
@2> function _pause() internal virtual whenNotPaused {
        PausableStorage storage $ = _getPausableStorage();
        $._paused = true;
        emit Paused(_msgSender());
    }

@1: emergencyWithdraw() attempts to pause the contract via _pause() @2: _pause() has a whenNotPaused modifier, which reverts if the contract is already paused

Root Cause

The root cause is the unconditional call to _pause() within emergencyWithdraw(), without checking if the contract is already paused. The PausableUpgradeable::_pause() function reverts when called on a contract that’s already paused, leading to an inability to execute emergencyWithdraw() when the contract is already paused.

Internal pre-conditions

No response

External pre-conditions

Someone with PAUSING_CONTRACTS_ROLE role pauses the contract before the admin attempts to make an emergency withdrawal

Attack Path

  • An emergency situation happens, such as a protocol exploit
  • Someone with PAUSING_CONTRACTS_ROLE role identifies the exploit before the admin and pauses the contract preemptively to stop the exploit
  • The DEFAULT_ADMIN_ROLE attempts to emergencyWithdraw() to safeguard the funds, but fails since the contract is already paused

Impact

This vulnerability prevents the admin from withdrawing funds in an emergency if the contract is paused. This means they would have to unpause the contract to safeguard the funds. This is critical, as the transaction to withdraw the funds after unpausing the contract could be frontrun by the attacker.

PoC

No response

Mitigation

Add a conditional check to ensure _pause() is only called if the contract is not already paused. This prevents reversion and ensures emergencyWithdraw() can function as intended during emergencies.

    function emergencyWithdraw(address safeAccount) external {
        Usd0PPStorageV0 storage $ = _usd0ppStorageV0();

        if (!$.registryAccess.hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) {
            revert NotAuthorized();
        }
        IERC20 usd0 = $.usd0;

        uint256 balance = usd0.balanceOf(address(this));
        // get the collateral token for the bond
        usd0.safeTransfer(safeAccount, balance);

        // Pause the contract
+       if (!paused()) {
+           _pause();
+       }
-       _pause();

        emit EmergencyWithdraw(safeAccount, balance);
    }