Fixes of the early results from the second OZ audit (#269)

* chore: audit fix L-01 * chore: audit fix L-02 * chore: audit fix N-01 * chore: audit fix N-02 * chore: audit fix N-04 * removed selectors in encodeCall * chore: linted
matter-labs · Feb 4, 2025 · a319811 · a319811
1 parent 6342e23
commit a319811
Show file tree

Hide file tree

Showing 9 changed files with 14 additions and 11 deletions.
diff --git a/src/SsoAccount.sol b/src/SsoAccount.sol
@@ -198,7 +198,7 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
     }
 
     // Extract the signature, validator address and hook data from the _transaction.signature
-    (bytes memory signature, address validator, ) = SignatureDecoder.decodeSignature(_transaction.signature);
+    (bytes memory signature, address validator) = SignatureDecoder.decodeSignatureNoHookData(_transaction.signature);
 
     bool validationSuccess = _isModuleValidator(validator) &&
       IModuleValidator(validator).validateTransaction(_signedHash, signature, _transaction);

diff --git a/src/interfaces/IHookManager.sol b/src/interfaces/IHookManager.sol
@@ -19,7 +19,7 @@ interface IHookManager {
   event HookRemoved(address indexed hook);
 
   /**
-   * @notice Add a hook to the list of hooks and call it's init function
+   * @notice Add a hook to the list of hooks and call its init function
    * @dev Can only be called by self
    * @param hook - Address of the hook
    * @param isValidation bool          - True if the hook is a validation hook, false otherwise

diff --git a/src/interfaces/IOwnerManager.sol b/src/interfaces/IOwnerManager.sol
@@ -10,13 +10,13 @@ interface IOwnerManager {
    * @notice Event emitted when a k1 owner is added
    * @param addr address - k1 owner that has been added
    */
-  event K1AddOwner(address indexed addr);
+  event K1OwnerAdded(address indexed addr);
 
   /**
    * @notice Event emitted when a k1 owner is removed
    * @param addr address - k1 owner that has been removed
    */
-  event K1RemoveOwner(address indexed addr);
+  event K1OwnerRemoved(address indexed addr);
 
   /**
    * @notice Adds a k1 owner to the list of k1 owners

diff --git a/src/interfaces/ISsoAccount.sol b/src/interfaces/ISsoAccount.sol
@@ -15,7 +15,7 @@ import { IValidatorManager } from "./IValidatorManager.sol";
 /**
  * @title ISsoAccount
  * @notice Interface for the SSO contract
- * @dev Implementations of this interface are contract that can be used as an SSO account (it's no longer Clave compatible)
+ * @dev Implementations of this interface are contracts that can be used as an SSO account (it's no longer Clave compatible)
  */
 interface ISsoAccount is
   IERC1271Upgradeable,

diff --git a/src/libraries/SessionLib.sol b/src/libraries/SessionLib.sol
@@ -286,7 +286,9 @@ library SessionLib {
       if (paymasterInputSelector == IPaymasterFlow.approvalBased.selector) {
         require(transaction.paymasterInput.length >= 68, "Invalid paymaster input length");
         (address token, uint256 amount, ) = abi.decode(transaction.paymasterInput[4:], (address, uint256, bytes));
-        bytes memory data = abi.encodeWithSelector(IERC20.approve.selector, transaction.paymaster, amount);
+        require(transaction.paymaster <= type(uint160).max, "Overflow");
+        address paymasterAddr = address(uint160(transaction.paymaster));
+        bytes memory data = abi.encodeCall(IERC20.approve, (paymasterAddr, amount));
 
         // check that session allows .approve() for this token
         CallSpec memory approvePolicy = checkCallPolicy(

diff --git a/src/libraries/SsoStorage.sol b/src/libraries/SsoStorage.sol
@@ -4,7 +4,8 @@ pragma solidity ^0.8.24;
 import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 
 library SsoStorage {
-  bytes32 private constant SSO_STORAGE_SLOT = 0x3248da1aeae8bd923cbf26901dc4bfc6bb48bb0fbc5b6102f1151fe7012884f4;
+  //keccak256('zksync-sso.contracts.SsoStorage')-1
+  bytes32 private constant SSO_STORAGE_SLOT = 0x996e49e905bb2c30d677a2ad554e4b964a479b19a0509deafafca5126b88ba23;
 
   struct Layout {
     // ┌───────────────────┐

diff --git a/src/managers/HookManager.sol b/src/managers/HookManager.sol
@@ -40,7 +40,7 @@ abstract contract HookManager is IHookManager, Auth {
   /// @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));
+    hook.excessivelySafeCall(gasleft(), 0, abi.encodeCall(IModule.onUninstall, (deinitData)));
   }
 
   /// @inheritdoc IHookManager

diff --git a/src/managers/OwnerManager.sol b/src/managers/OwnerManager.sol
@@ -40,13 +40,13 @@ abstract contract OwnerManager is IOwnerManager, Auth {
   function _k1AddOwner(address addr) internal {
     require(_k1Owners().add(addr), "K1 owner already exists");
 
-    emit K1AddOwner(addr);
+    emit K1OwnerAdded(addr);
   }
 
   function _k1RemoveOwner(address addr) internal {
     require(_k1Owners().remove(addr), "K1 owner not found");
 
-    emit K1RemoveOwner(addr);
+    emit K1OwnerRemoved(addr);
   }
 
   function _k1IsOwner(address addr) internal view returns (bool) {

diff --git a/src/managers/ValidatorManager.sol b/src/managers/ValidatorManager.sol
@@ -39,7 +39,7 @@ abstract contract ValidatorManager is IValidatorManager, Auth {
   ///@inheritdoc IValidatorManager
   function unlinkModuleValidator(address validator, bytes calldata deinitData) external onlySelf {
     _removeModuleValidator(validator);
-    validator.excessivelySafeCall(gasleft(), 0, abi.encodeWithSelector(IModule.onUninstall.selector, deinitData));
+    validator.excessivelySafeCall(gasleft(), 0, abi.encodeCall(IModule.onUninstall, (deinitData)));
   }
 
   /// @inheritdoc IValidatorManager