HIGH severity: Lack of Output Validation Can Lock the SsoAccount. The SsoAccount contract and its managers use OpenZeppelin's EnumerableSet library to manage sets of hooks, owners, and validators. However, the code does not check the boolean return values of the `add` and `remove` functions, incorrectly assuming that these operations will revert on failure. This l...

Vulnerable code:
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.24;

import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { Transaction } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/TransactionHelper.sol";
import { ExcessivelySafeCall } from "@nomad-xyz/excessively-safe-call/src/ExcessivelySafeCall.sol";

import { Auth } from "../auth/Auth.sol";
import { SsoStorage } from "../libraries/SsoStorage.sol";
import { Errors } from "../libraries/Errors.sol";
import { IExecutionHook, IValidationHook } from "../interfaces/IHook.sol";
import { IHookManager } from "../interfaces/IHookManager.sol";
import { IModule } from "../interfaces/IModule.sol";

/**
 * @title Manager contract for hooks
 * @notice Abstract contract for managing the enabled hooks of the account
 * @dev Hook addresses are stored in a linked list
 * @author https://getclave.io
 */
abstract contract HookManager is IHookManager, Auth {
  using EnumerableSet for EnumerableSet.AddressSet;
  // Interface helper library
  using ERC165Checker for address;
  // Low level calls helper library
  using ExcessivelySafeCall for address;

  /// @inheritdoc IHookManager
  function addHook(address hook, bool isValidation, bytes calldata initData) external override onlySelf {
    _addHook(hook, isValidation, initData);
  }

  /// @inheritdoc IHookManager
  function removeHook(address hook, bool isValidation, bytes calldata deinitData) external override onlySelf {
    _removeHook(hook, isValidation);
    IModule(hook).onUninstall(deinitData);
  }

  /// @inheritdoc IHookManager
  function unlinkHook(address hook, bool isValidation, bytes calldata deinitData) external onlySelf {
    _removeHook(hook, isValidation);
    hook.excessivelySafeCall(gasleft(), 0, abi.encodeWithSelector(IModule.onUninstall.selector, deinitData));
  }

  /// @inheritdoc IHookManager
  function isHook(address addr) external view override returns (bool) {
    return _isHook(addr);
  }

  /// @inheritdoc IHookManager
  function listHooks(bool isValidation) external view override returns (address[] memory hookList) {
    if (isValidation) {
      hookList = _validationHooks().values();
    } else {
      hookList = _executionHooks().values();
    }
  }

  // Runs the validation hooks that are enabled by the account and returns true if none reverts
  function runValidationHooks(bytes32 signedHash, Transaction calldata transaction) internal returns (bool) {
    EnumerableSet.AddressSet storage hookList = _validationHooks();
    uint256 totalHooks = hookList.length();

    for (uint256 i = 0; i < totalHooks; i++) {
      bool success = _call(hookList.at(i), abi.encodeCall(IValidationHook.validationHook, (signedHash, transaction)));

      if (!success) {
        return false;
      }
    }

    return true;
  }

  // Runs the execution hooks that are enabled by the account before and after _executeTransaction
  modifier runExecutionHooks(Transaction calldata transaction) {
    EnumerableSet.AddressSet storage hookList = _executionHooks();
    uint256 totalHooks = hookList.length();

    for (uint256 i = 0; i < totalHooks; i++) {
      IExecutionHook(hookList.at(i)).preExecutionHook(transaction);
    }

    _;

    for (uint256 i = 0; i < totalHooks; i++) {
      IExecutionHook(hookList.at(i)).postExecutionHook();
    }
  }

  function _addHook(address hook, bool isValidation, bytes calldata initData) internal {
    if (!_supportsHook(hook, isValidation)) {
      revert Errors.HOOK_ERC165_FAIL(hook, isValidation);
    }

    if (isValidation) {
      _validationHooks().add(hook);
    } else {
      _executionHooks().add(hook);
    }

    IModule(hook).onInstall(initData);

    emit HookAdded(hook);
  }

  function _removeHook(address hook, bool isValidation) internal {
    if (isValidation) {
      _validationHooks().remove(hook);
    } else {
      _executionHooks().remove(hook);
    }

    emit HookRemoved(hook);
  }

  function _isHook(address addr) internal view override returns (bool) {
    return _validationHooks().contains(addr) || _executionHooks().contains(addr);
  }

  function _call(address target, bytes memory data) private returns (bool success) {
    assembly ("memory-safe") {
      success := call(gas(), target, 0, add(data, 0x20), mload(data), 0, 0)
    }
  }

  function _validationHooks() private view returns (EnumerableSet.AddressSet storage validationHooks) {
    validationHooks = SsoStorage.layout().validationHooks;
  }

  function _executionHooks() private view returns (EnumerableSet.AddressSet storage executionHooks) {
    executionHooks = SsoStorage.layout().executionHooks;
  }

  function _supportsHook(address hook, bool isValidation) private view returns (bool) {
    return
      hook.supportsInterface(type(IModule).interfaceId) &&
      (
        isValidation
          ? hook.supportsInterface(type(IValidationHook).interfaceId)
          : hook.supportsInterface(type(IExecutionHook).interfaceId)
      );
  }
}

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.24;

import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { SsoStorage } from "../libraries/SsoStorage.sol";
import { Errors } from "../libraries/Errors.sol";
import { Auth } from "../auth/Auth.sol";
import { IOwnerManager } from "../interfaces/IOwnerManager.sol";

/**
 * @title Manager contract for owners
 * @notice Abstract contract for managing the owners of the account
 * @dev K1 Owners are secp256k1 addresses
 * @dev Owners are stored in a linked list
 * @author https://getclave.io
 */
abstract contract OwnerManager is IOwnerManager, Auth {
  using EnumerableSet for EnumerableSet.AddressSet;

  /// @inheritdoc IOwnerManager
  function k1AddOwner(address addr) external override onlySelf {
    _k1AddOwner(addr);
  }

  /// @inheritdoc IOwnerManager
  function k1RemoveOwner(address addr) external override onlySelf {
    _k1RemoveOwner(addr);
  }

  /// @inheritdoc IOwnerManager
  function k1IsOwner(address addr) external view override returns (bool) {
    return _k1IsOwner(addr);
  }

  /// @inheritdoc IOwnerManager
  function k1ListOwners() external view override returns (address[] memory k1OwnerList) {
    k1OwnerList = _k1Owners().values();
  }

  function _k1AddOwner(address addr) internal {
    _k1Owners().add(addr);

    emit K1AddOwner(addr);
  }

  function _k1RemoveOwner(address addr) internal {
    _k1Owners().remove(addr);

    emit K1RemoveOwner(addr);
  }

  function _k1IsOwner(address addr) internal view returns (bool) {
    return _k1Owners().contains(addr);
  }

  function _k1Owners() private view returns (EnumerableSet.AddressSet storage k1Owners) {
    k1Owners = SsoStorage.layout().k1Owners;
  }
}

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.24;

import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { ExcessivelySafeCall } from "@nomad-xyz/excessively-safe-call/src/ExcessivelySafeCall.sol";

import { Auth } from "../auth/Auth.sol";
import { Errors } from "../libraries/Errors.sol";
import { SsoStorage } from "../libraries/SsoStorage.sol";
import { IValidatorManager } from "../interfaces/IValidatorManager.sol";
import { IModuleValidator } from "../interfaces/IModuleValidator.sol";
import { IModule } from "../interfaces/IModule.sol";

/**
 * @title Manager contract for validators
 * @notice Abstract contract for managing the validators of the account
 * @dev Validators are stored in an enumerable set
 * @author https://getclave.io
 */
abstract contract ValidatorManager is IValidatorManager, Auth {
  using EnumerableSet for EnumerableSet.AddressSet;
  // Interface helper library
  using ERC165Checker for address;
  // Low level calls helper library
  using ExcessivelySafeCall for address;

  ///@inheritdoc IValidatorManager
  function addModuleValidator(address validator, bytes calldata initData) external onlySelf {
    _addModuleValidator(validator, initData);
  }

  ///@inheritdoc IValidatorManager
  function removeModuleValidator(address validator, bytes calldata deinitData) external onlySelf {
    _removeModuleValidator(validator);
    IModule(validator).onUninstall(deinitData);
  }

  ///@inheritdoc IValidatorManager
  function unlinkModuleValidator(address validator, bytes calldata deinitData) external onlySelf {
    _removeModuleValidator(validator);
    validator.excessivelySafeCall(gasleft(), 0, abi.encodeWithSelector(IModule.onUninstall.selector, deinitData));
  }

  /// @inheritdoc IValidatorManager
  function isModuleValidator(address validator) external view override returns (bool) {
    return _isModuleValidator(validator);
  }

  /// @inheritdoc IValidatorManager
  function listModuleValidators() external view override returns (address[] memory validatorList) {
    validatorList = _moduleValidators().values();
  }

  function _addModuleValidator(address validator, bytes memory initData) internal {
    if (!_supportsModuleValidator(validator)) {
      revert Errors.VALIDATOR_ERC165_FAIL(validator);
    }

    _moduleValidators().add(validator);
    IModule(validator).onInstall(initData);

    emit ValidatorAdded(validator);
  }

  function _removeModuleValidator(address validator) internal {
    _moduleValidators().remove(validator);

    emit ValidatorRemoved(validator);
  }

  function _isModuleValidator(address validator) internal view returns (bool) {
    return _moduleValidators().contains(validator);
  }

  function _supportsModuleValidator(address validator) private view returns (bool) {
    return
      validator.supportsInterface(type(IModuleValidator).interfaceId) &&
      validator.supportsInterface(type(IModule).interfaceId);
  }

  function _moduleValidators() private view returns (EnumerableSet.AddressSet storage moduleValidators) {
    moduleValidators = SsoStorage.layout().moduleValidators;
  }
}


Vulnerability type: CWE-703|CWE-754|CWE-252