Add audit fixes

.solhint.json

@@ -13,6 +13,7 @@
     "no-empty-blocks": "off",
     "not-rely-on-time": "off",
     "one-contract-per-file": "off",
-    "var-name-mixedcase": "off"
+    "var-name-mixedcase": "off",
+    "immutable-vars-naming": "off"
   }
 }

src/EmailRecoveryManager.sol

@@ -441,17 +441,27 @@ abstract contract EmailRecoveryManager is
     }
 
     /**
-     * @notice Removes all state related to an account.
+     * @notice Removes all state related to msg.sender.
      * @dev In order to prevent unexpected behaviour when reinstalling account modules, the module
      * should be deinitialized. This should include remove state accociated with an account.
      */
     function deInitRecoveryModule() internal onlyWhenNotRecovering {
-        delete recoveryConfigs[msg.sender];
-        delete recoveryRequests[msg.sender];
+        address account = msg.sender;
+        deInitRecoveryModule(account);
+    }
+
+    /**
+     * @notice Removes all state related to an account.
+     * @dev In order to prevent unexpected behaviour when reinstalling account modules, the module
+     * should be deinitialized. This should include remove state accociated with an account.
+     */
+    function deInitRecoveryModule(address account) internal onlyWhenNotRecovering {
+        delete recoveryConfigs[account];
+        delete recoveryRequests[account];
 
-        removeAllGuardians(msg.sender);
-        delete guardianConfigs[msg.sender];
+        removeAllGuardians(account);
+        delete guardianConfigs[account];
 
-        emit RecoveryDeInitialized(msg.sender);
+        emit RecoveryDeInitialized(account);
     }
 }

src/handlers/EmailRecoverySubjectHandler.sol

@@ -124,7 +124,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler {
         bytes[] calldata subjectParams
     )
         public
-        view
+        pure
         returns (address)
     {
         if (templateIdx != 0) {

src/interfaces/IEmailRecoveryManager.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.25;
 
-import { GuardianStorage, GuardianStatus } from "../libraries/EnumerableGuardianMap.sol";
+import { GuardianStatus } from "../libraries/EnumerableGuardianMap.sol";
 
 interface IEmailRecoveryManager {
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/

src/interfaces/ISafe.sol

@@ -14,5 +14,9 @@ interface ISafe {
     )
         external
         returns (bool success);
-     function isModuleEnabled(address module) external view returns (bool);
+    function isModuleEnabled(address module) external view returns (bool);
+    function addOwnerWithThreshold(address owner, uint256 _threshold) external;
+    function removeOwner(address prevOwner, address owner, uint256 _threshold) external;
+    function swapOwner(address prevOwner, address oldOwner, address newOwner) external;
+    function changeThreshold(uint256 _threshold) external;
 }

src/modules/EmailRecoveryModule.sol

@@ -7,7 +7,6 @@ import { IModule } from "erc7579/interfaces/IERC7579Module.sol";
 import { ISafe } from "../interfaces/ISafe.sol";
 import { IEmailRecoveryModule } from "../interfaces/IEmailRecoveryModule.sol";
 import { EmailRecoveryManager } from "../EmailRecoveryManager.sol";
-import { GuardianManager } from "../GuardianManager.sol";
 
 /**
  * @title EmailRecoveryModule
@@ -53,13 +52,21 @@ contract EmailRecoveryModule is EmailRecoveryManager, ERC7579ExecutorBase, IEmai
         if (_validator == address(0)) {
             revert InvalidValidator(_validator);
         }
-        if (
-            _selector == IModule.onUninstall.selector || _selector == IModule.onInstall.selector
-                || _selector == IERC7579Account.execute.selector
-                || _selector == ISafe.setFallbackHandler.selector
-                || _selector == ISafe.setGuard.selector || _selector == bytes4(0)
-        ) {
-            revert InvalidSelector(_selector);
+        if(_validator == msg.sender) {
+            if (
+                _selector != ISafe.addOwnerWithThreshold.selector && _selector != ISafe.removeOwner.selector 
+                && _selector != ISafe.swapOwner.selector 
+                && _selector != ISafe.changeThreshold.selector 
+            ) {
+                revert InvalidSelector(_selector);
+            }
+        } else {
+            if (
+                _selector == IModule.onInstall.selector || _selector == IModule.onUninstall.selector
+                || _selector == bytes4(0)
+            ) {
+                revert InvalidSelector(_selector);
+            }
         }
 
         validator = _validator;
@@ -140,14 +147,11 @@ contract EmailRecoveryModule is EmailRecoveryManager, ERC7579ExecutorBase, IEmai
      * being recovered. recoveryData = abi.encode(validator, recoveryFunctionCalldata)
      */
     function recover(address account, bytes calldata recoveryData) internal override {
-        (address validator, bytes memory recoveryCalldata) =
+        (, bytes memory recoveryCalldata) =
             abi.decode(recoveryData, (address, bytes));
 
-        if (validator == address(0)) {
-            revert InvalidValidator(validator);
-        }
-
         bytes4 calldataSelector;
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             calldataSelector := mload(add(recoveryCalldata, 32))
         }

src/modules/SafeEmailRecoveryModule.sol

@@ -61,6 +61,7 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
         }
 
         bytes4 calldataSelector;
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             calldataSelector := mload(add(recoveryCalldata, 32))
         }
@@ -92,6 +93,6 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
         if (ISafe(account).isModuleEnabled(address(this)) == true) {
             revert ResetFailed(account);
         }
-        deInitRecoveryModule();
+        deInitRecoveryModule(account);
     }
 }

src/modules/UniversalEmailRecoveryModule.sol

@@ -5,7 +5,7 @@ import { ERC7579ExecutorBase } from "@rhinestone/modulekit/src/Modules.sol";
 import { IERC7579Account } from "erc7579/interfaces/IERC7579Account.sol";
 import { IModule } from "erc7579/interfaces/IERC7579Module.sol";
 import { ISafe } from "../interfaces/ISafe.sol";
-import { SentinelListLib, SENTINEL, ZERO_ADDRESS } from "sentinellist/SentinelList.sol";
+import { SentinelListLib, SENTINEL } from "sentinellist/SentinelList.sol";
 import { IUniversalEmailRecoveryModule } from "../interfaces/IUniversalEmailRecoveryModule.sol";
 import { EmailRecoveryManager } from "../EmailRecoveryManager.sol";
 
@@ -68,14 +68,22 @@ contract UniversalEmailRecoveryModule is
      * @notice Modifier to check whether the selector is safe. Reverts if the selector is for
      * "onInstall" or "onUninstall"
      */
-    modifier withoutUnsafeSelector(bytes4 selector) {
-        if (
-            selector == IModule.onUninstall.selector || selector == IModule.onInstall.selector
-                || selector == IERC7579Account.execute.selector
-                || selector == ISafe.setFallbackHandler.selector || selector == ISafe.setGuard.selector
+    modifier withoutUnsafeSelector(address validator, bytes4 selector) {
+         if(validator == msg.sender) {
+            if (
+                selector != ISafe.addOwnerWithThreshold.selector && selector != ISafe.removeOwner.selector 
+                && selector != ISafe.swapOwner.selector 
+                && selector != ISafe.changeThreshold.selector 
+            ) {
+                revert InvalidSelector(selector);
+            }
+        } else {
+            if (
+                selector == IModule.onInstall.selector || selector == IModule.onUninstall.selector
                 || selector == bytes4(0)
-        ) {
-            revert InvalidSelector(selector);
+            ) {
+                revert InvalidSelector(selector);
+            }
         }
         _;
     }
@@ -149,7 +157,7 @@ contract UniversalEmailRecoveryModule is
     )
         public
         onlyWhenInitialized
-        withoutUnsafeSelector(recoverySelector)
+        withoutUnsafeSelector(validator, recoverySelector)
     {
         if (
             !IERC7579Account(msg.sender).isModuleInstalled(
@@ -270,6 +278,7 @@ contract UniversalEmailRecoveryModule is
         }
 
         bytes4 selector;
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             selector := mload(add(recoveryCalldata, 32))
         }

test/unit/UnitBase.t.sol

@@ -28,7 +28,7 @@ import { EmailRecoveryFactory } from "src/factories/EmailRecoveryFactory.sol";
 import { EmailRecoveryUniversalFactory } from "src/factories/EmailRecoveryUniversalFactory.sol";
 import { OwnableValidator } from "src/test/OwnableValidator.sol";
 import { MockGroth16Verifier } from "src/test/MockGroth16Verifier.sol";
-import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
+import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
 abstract contract UnitBase is RhinestoneModuleKit, Test {
     using ModuleKitHelpers for *;

test/unit/modules/EmailRecoveryModule/constructor.t.sol

@@ -7,6 +7,7 @@ import { IERC7579Account } from "erc7579/interfaces/IERC7579Account.sol";
 import { ISafe } from "src/interfaces/ISafe.sol";
 import { EmailRecoveryModuleBase } from "./EmailRecoveryModuleBase.t.sol";
 import { EmailRecoveryModule } from "src/modules/EmailRecoveryModule.sol";
+import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
 
 contract EmailRecoveryModule_constructor_Test is EmailRecoveryModuleBase {
     function setUp() public override {
@@ -28,42 +29,59 @@ contract EmailRecoveryModule_constructor_Test is EmailRecoveryModuleBase {
         );
     }
 
-    function test_Constructor_RevertWhen_UnsafeOnInstallSelector() public {
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                EmailRecoveryModule.InvalidSelector.selector, IModule.onInstall.selector
-            )
+    function test_Constructor_When_SafeAddOwnerSelector() public {
+        _skipIfNotSafeAccountType();
+        new EmailRecoveryModule(
+            address(verifier),
+            address(dkimRegistry),
+            address(emailAuthImpl),
+            address(emailRecoveryHandler),
+            validatorAddress,
+            ISafe.addOwnerWithThreshold.selector
         );
+    }
+
+    function test_Constructor_When_SafeRemoveOwnerSelector() public {
+        _skipIfNotSafeAccountType();
         new EmailRecoveryModule(
             address(verifier),
             address(dkimRegistry),
             address(emailAuthImpl),
             address(emailRecoveryHandler),
             validatorAddress,
-            IModule.onInstall.selector
+            ISafe.removeOwner.selector
         );
     }
 
-    function test_Constructor_RevertWhen_UnsafeOnUninstallSelector() public {
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                EmailRecoveryModule.InvalidSelector.selector, IModule.onUninstall.selector
-            )
+    function test_Constructor_When_SafeSwapOwnerSelector() public {
+        _skipIfNotSafeAccountType();
+        new EmailRecoveryModule(
+            address(verifier),
+            address(dkimRegistry),
+            address(emailAuthImpl),
+            address(emailRecoveryHandler),
+            validatorAddress,
+            ISafe.swapOwner.selector
         );
+    }
+
+    function test_Constructor_When_SafeChangeThresholdSelector() public {
+        _skipIfNotSafeAccountType();
         new EmailRecoveryModule(
             address(verifier),
             address(dkimRegistry),
             address(emailAuthImpl),
             address(emailRecoveryHandler),
             validatorAddress,
-            IModule.onUninstall.selector
+            ISafe.changeThreshold.selector
         );
     }
 
-    function test_Constructor_RevertWhen_UnsafeExecuteSelector() public {
+    function test_Constructor_RevertWhen_SafeNotAllowedSelector() public {
+        _skipIfNotSafeAccountType();
         vm.expectRevert(
             abi.encodeWithSelector(
-                EmailRecoveryModule.InvalidSelector.selector, IERC7579Account.execute.selector
+                EmailRecoveryModule.InvalidSelector.selector, IModule.onInstall.selector
             )
         );
         new EmailRecoveryModule(
@@ -72,14 +90,14 @@ contract EmailRecoveryModule_constructor_Test is EmailRecoveryModuleBase {
             address(emailAuthImpl),
             address(emailRecoveryHandler),
             validatorAddress,
-            IERC7579Account.execute.selector
+            IModule.onInstall.selector
         );
     }
 
-    function test_Constructor_RevertWhen_UnsafeSetFallbackHandlerSelector() public {
+    function test_Constructor_RevertWhen_UnsafeOnInstallSelector() public {
         vm.expectRevert(
             abi.encodeWithSelector(
-                EmailRecoveryModule.InvalidSelector.selector, ISafe.setFallbackHandler.selector
+                EmailRecoveryModule.InvalidSelector.selector, IModule.onInstall.selector
             )
         );
         new EmailRecoveryModule(
@@ -88,14 +106,14 @@ contract EmailRecoveryModule_constructor_Test is EmailRecoveryModuleBase {
             address(emailAuthImpl),
             address(emailRecoveryHandler),
             validatorAddress,
-            ISafe.setFallbackHandler.selector
+            IModule.onInstall.selector
         );
     }
 
-    function test_Constructor_RevertWhen_UnsafeSetGuardSelector() public {
+    function test_Constructor_RevertWhen_UnsafeOnUninstallSelector() public {
         vm.expectRevert(
             abi.encodeWithSelector(
-                EmailRecoveryModule.InvalidSelector.selector, ISafe.setGuard.selector
+                EmailRecoveryModule.InvalidSelector.selector, IModule.onUninstall.selector
             )
         );
         new EmailRecoveryModule(
@@ -104,7 +122,7 @@ contract EmailRecoveryModule_constructor_Test is EmailRecoveryModuleBase {
             address(emailAuthImpl),
             address(emailRecoveryHandler),
             validatorAddress,
-            ISafe.setGuard.selector
+            IModule.onUninstall.selector
         );
     }
 
@@ -135,4 +153,13 @@ contract EmailRecoveryModule_constructor_Test is EmailRecoveryModuleBase {
         assertEq(validatorAddress, emailRecoveryModule.validator());
         assertEq(functionSelector, emailRecoveryModule.selector());
     }
+
+    function _skipIfNotSafeAccountType() private {
+        string memory currentAccountType = vm.envOr("ACCOUNT_TYPE", string(""));
+        if (Strings.equal(currentAccountType, "SAFE")) {
+            vm.skip(false);
+        } else {
+            vm.skip(true);
+        }
+    }
 }