fix: STAA-14 remediation commit i

Author: livingrockrises

src/StartaleSmartAccount.sol

@@ -38,6 +38,8 @@ import {
 } from './types/Constants.sol';
 
 import {EmergencyUninstall} from './types/Structs.sol';
+
+import {_packValidationData} from '@account-abstraction/core/Helpers.sol';
 import {PackedUserOperation} from '@account-abstraction/interfaces/PackedUserOperation.sol';
 import {SENTINEL, SentinelListLib, ZERO_ADDRESS} from 'sentinellist/SentinelList.sol';
 import {UUPSUpgradeable} from 'solady/utils/UUPSUpgradeable.sol';
@@ -113,7 +115,11 @@ contract StartaleSmartAccount is
     } else if (op.nonce.isModuleEnableMode()) {
       // if it is module enable mode, we need to enable the module first
       // and get the cleaned signature
-      userOp.signature = _enableMode(userOpHash, op.signature);
+      (bool enableModeSigValid, bytes calldata userOpSignature) = _enableMode(userOpHash, op.signature);
+      if (!enableModeSigValid) {
+        return _packValidationData(true, 0, 0);
+      }
+      userOp.signature = userOpSignature;
     }
     validator = _handleValidator(op.nonce.getValidator());
     (userOpHash, userOp.signature) = _withPreValidationHook(userOpHash, userOp, missingAccountFunds);

src/core/ModuleManager.sol

@@ -141,8 +141,12 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
 
   /// @dev Implements Module Enable Mode flow.
   /// @param packedData Data source to parse data required to perform Module Enable mode from.
+  /// @return enableModeSigValid true if the enable mode signature is valid, false otherwise
   /// @return userOpSignature the clean signature which can be further used for userOp validation
-  function _enableMode(bytes32 userOpHash, bytes calldata packedData) internal returns (bytes calldata userOpSignature) {
+  function _enableMode(
+    bytes32 userOpHash,
+    bytes calldata packedData
+  ) internal returns (bool enableModeSigValid, bytes calldata userOpSignature) {
     address module;
     uint256 moduleType;
     bytes calldata moduleInitData;
@@ -161,9 +165,11 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
         validator: enableModeSigValidator
       })
     ) {
-      revert EnableModeSigError();
+      // Review
+      return (false, userOpSignature);
     }
     this.installModule{value: msg.value}(moduleType, module, moduleInitData);
+    return (true, userOpSignature);
   }
 
   /// @notice Installs a new module to the smart account.
@@ -578,11 +584,7 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
     // Even if the validator doesn't support 7739 under the hood, it is still secure,
     // as eip712digest is already built based on 712Domain of this Smart Account
     // This interface should always be exposed by validators as per ERC-7579
-    try IValidator(validator).isValidSignatureWithSender(address(this), eip712Digest, sig) returns (bytes4 res) {
-      return res == ERC1271_MAGICVALUE;
-    } catch {
-      return false;
-    }
+    return IValidator(validator).isValidSignatureWithSender(address(this), eip712Digest, sig) == ERC1271_MAGICVALUE;
   }
 
   /// @notice Builds the enable mode data hash as per eip712

test/foundry/unit/concrete/modulemanager/TestModuleManager_EnableMode.t.sol

@@ -492,9 +492,7 @@ contract TestModuleManager_EnableMode is Test, TestModuleManagerBase {
     PackedUserOperation[] memory userOps = new PackedUserOperation[](1);
     userOps[0] = op;
 
-    bytes memory expectedRevertReason = abi.encodeWithSelector(
-      FailedOpWithRevert.selector, 0, 'AA23 reverted', abi.encodeWithSelector(EnableModeSigError.selector)
-    );
+    bytes memory expectedRevertReason = abi.encodeWithSelector(FailedOpWithRevert.selector, 0, 'AA23 reverted', 0x0);
 
     vm.expectRevert(expectedRevertReason);
     ENTRYPOINT.handleOps(userOps, payable(BOB.addr));