IRREVERSIBLE SHUTDOWN FUNCTION

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/main/src/VaultImplementation.sol#L146-L150

Vulnerability details

Impact

The abstract contract VaultImplementation is inherited by Vault.sol and PublicVault.sol. Its shutdown() can be called by the strategist to make _loadVISlot().isShutdown = true. This will make the modifier whenNotPaused() revert, rendering commitToLien() and buyoutLien() unable to execute forever because no where in the contract can be found a function to undo the shutdown.

Proof of Concept

The second if block of the following modifier is dependent on _loadVISlot().isShutdown:

VaultImplementation.sol#L131-L140

  modifier whenNotPaused() {
    if (ROUTER().paused()) {
      revert InvalidRequest(InvalidRequestReason.PAUSED);
    }

    if (_loadVISlot().isShutdown) {
      revert InvalidRequest(InvalidRequestReason.SHUTDOWN);
    }
    _;
  }

The above modifier is going to revert with the following function invoked:

VaultImplementation.sol#L146-L150

  function shutdown() external {
    require(msg.sender == owner()); //owner is "strategist"
    _loadVISlot().isShutdown = true;
    emit VaultShutdown();
  }

Consequently, the following two functions whose visibility includes whenNotPaused will be non-callable:

VaultImplementation.sol#L287-L306

  function commitToLien(
    IAstariaRouter.Commitment calldata params,
    address receiver
  )
    external
    whenNotPaused
    returns (uint256 lienId, ILienToken.Stack[] memory stack, uint256 payout)
  { ... }

VaultImplementation.sol#L313-L330

  function buyoutLien(
    ILienToken.Stack[] calldata stack,
    uint8 position,
    IAstariaRouter.Commitment calldata incomingTerms
  )
    external
    whenNotPaused
    returns (ILienToken.Stack[] memory, ILienToken.Stack memory)
  { ... }

Recommended Mitigation Steps

It is recommended implementing unShutdown() in the contract with opposite logic to shutdown() so that the strategist gets the option to reverse whenNotPaused.

[Picodes]

It seems legitimate to have an unreversible shutdown function. What is the issue with this? You need to expand on the impact.

[c4-judge]

Picodes marked the issue as unsatisfactory: Insufficient proof