fix: L04 and I02 from Certora audit

KintoXYZ · Jan 29, 2024 · fcf4e1c · fcf4e1c
1 parent a54f039
commit fcf4e1c
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 18 deletions.
diff --git a/script/deploy.sol b/script/deploy.sol
@@ -75,7 +75,7 @@ contract KintoInitialDeployScript is Create2Helper, ArtifactsReader {
         _kintoID = KintoID(address(_proxy));
 
         // Entry Point
-        address entryPointAddr = computeAddress(1, abi.encodePacked(type(EntryPoint).creationCode));
+        address entryPointAddr = computeAddress(bytes32(uint256(1)), abi.encodePacked(type(EntryPoint).creationCode));
         // Check Entry Point
         if (isContract(entryPointAddr)) {
             _entryPoint = EntryPoint(payable(entryPointAddr));

diff --git a/script/test.sol b/script/test.sol
@@ -48,7 +48,7 @@ contract KintoDeployTestWalletScript is AASetup, KYCSignature {
         }
 
         console.log("This factory has", totalWalletsCreated, " created");
-        uint256 salt = 0;
+        bytes32 salt = 0;
         address newWallet = _walletFactory.getAddress(recipientWallet, recipientWallet, salt);
         if (isContract(newWallet)) {
             console.log("Wallet already deployed for owner", recipientWallet, "at", newWallet);
@@ -112,7 +112,7 @@ contract KintoDeployTestCounter is AASetup, KYCSignature, UserOp {
         address deployerPublicKey = vm.rememberKey(vm.envUint("TEST_PRIVATE_KEY"));
         console.log("All AA setup is correct");
         vm.startBroadcast(deployerPrivateKey);
-        uint256 salt = 0;
+        bytes32 salt = 0;
         address newWallet = _walletFactory.getAddress(deployerPublicKey, deployerPublicKey, salt);
         if (!isContract(newWallet)) {
             console.log("ERROR: Wallet not deployed for owner", deployerPublicKey, "at", newWallet);
@@ -184,7 +184,7 @@ contract KintoDeployETHPriceIsRight is AASetup, KYCSignature, UserOp {
         address deployerPublicKey = vm.rememberKey(vm.envUint("TEST_PRIVATE_KEY"));
         console.log("All AA setup is correct");
         vm.startBroadcast(deployerPrivateKey);
-        uint256 salt = 0;
+        bytes32 salt = 0;
         address newWallet = _walletFactory.getAddress(deployerPublicKey, deployerPublicKey, salt);
         if (!isContract(newWallet)) {
             console.log("ERROR: Wallet not deployed for owner", deployerPublicKey, "at", newWallet);

diff --git a/src/KintoID.sol b/src/KintoID.sol
@@ -39,11 +39,13 @@ contract KintoID is
     event AccountsMonitoredAt(address indexed _signer, uint256 _accountsCount, uint256 _timestamp);
 
     /* ============ Constants ============ */
+
     bytes32 public constant override KYC_PROVIDER_ROLE = keccak256("KYC_PROVIDER_ROLE");
     bytes32 public constant override UPGRADER_ROLE = keccak256("UPGRADER_ROLE");
 
     /* ============ State Variables ============ */
 
+    bytes32 public domainSeparator;
     uint256 private _nextTokenId;
 
     // We'll monitor the whole list every single day and update it
@@ -71,10 +73,13 @@ contract KintoID is
         __ERC721Burnable_init();
         __AccessControl_init();
         __UUPSUpgradeable_init();
+
         _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
         _grantRole(KYC_PROVIDER_ROLE, msg.sender);
         _grantRole(UPGRADER_ROLE, msg.sender);
+
         lastMonitoredAt = block.timestamp;
+        domainSeparator = _domainSeparator();
     }
 
     /**
@@ -412,17 +417,12 @@ contract KintoID is
         }
         require(size == 0, "Signer must be an EOA");
 
-        bytes32 eip712MessageHash = _getEIP712Message(_signature);
+        bytes32 eip712MessageHash =
+            keccak256(abi.encodePacked("\x19\x01", domainSeparator, _hashSignatureData(_signature)));
         require(_signature.signer.isValidSignatureNow(eip712MessageHash, _signature.signature), "Invalid Signer");
         _;
     }
 
-    function _getEIP712Message(SignatureData memory signatureData) internal view returns (bytes32) {
-        bytes32 domainSeparator = _domainSeparator();
-        bytes32 structHash = _hashSignatureData(signatureData);
-        return keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
-    }
-
     /* ============ EIP-712 Helpers ============ */
 
     function _domainSeparator() internal view returns (bytes32) {

diff --git a/src/interfaces/IKintoWalletFactory.sol b/src/interfaces/IKintoWalletFactory.sol
@@ -13,7 +13,7 @@ interface IKintoWalletFactory {
 
     function upgradeAllWalletImplementations(IKintoWallet newImplementationWallet) external;
 
-    function createAccount(address owner, address recoverer, uint256 salt) external returns (IKintoWallet ret);
+    function createAccount(address owner, address recoverer, bytes32 salt) external returns (IKintoWallet ret);
 
     function deployContract(address contractOwner, uint256 amount, bytes memory bytecode, bytes32 salt)
         external
@@ -34,7 +34,7 @@ interface IKintoWalletFactory {
 
     /* ============ Basic Viewers ============ */
 
-    function getAddress(address owner, address recoverer, uint256 salt) external view returns (address);
+    function getAddress(address owner, address recoverer, bytes32 salt) external view returns (address);
 
     function getContractAddress(bytes32 salt, bytes32 bytecodeHash) external view returns (address);
 

diff --git a/src/wallet/KintoWalletFactory.sol b/src/wallet/KintoWalletFactory.sol
@@ -88,7 +88,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
      * @param salt The salt to use for the calculation
      * @return ret address of the account
      */
-    function createAccount(address owner, address recoverer, uint256 salt)
+    function createAccount(address owner, address recoverer, bytes32 salt)
         external
         override
         returns (IKintoWallet ret)
@@ -104,15 +104,15 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
 
         ret = IKintoWallet(
             payable(
-                new SafeBeaconProxy{salt: bytes32(salt)}(
+                new SafeBeaconProxy{salt: salt}(
                     address(beacon), abi.encodeCall(IKintoWallet.initialize, (owner, recoverer))
                 )
             )
         );
 
         walletTs[address(ret)] = block.timestamp;
         totalWallets++;
-        // Emit event
+
         emit KintoWalletFactoryCreation(address(ret), owner, factoryWalletVersion);
     }
 
@@ -230,7 +230,7 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
      * @param salt The salt to use for the calculation
      * @return The address of the account
      */
-    function getAddress(address owner, address recoverer, uint256 salt) public view override returns (address) {
+    function getAddress(address owner, address recoverer, bytes32 salt) public view override returns (address) {
         return Create2.computeAddress(
             bytes32(salt),
             keccak256(

diff --git a/test/helpers/Create2Helper.sol b/test/helpers/Create2Helper.sol
@@ -8,7 +8,7 @@ abstract contract Create2Helper {
     address CREATE2_DEPLOYER = 0x4e59b44847b379578588920cA78FbF26c0B4956C;
 
     /// @notice Precompute a contract address deployed via CREATE2
-    function computeAddress(uint256 salt, bytes memory creationCode) internal view returns (address) {
+    function computeAddress(bytes32 salt, bytes memory creationCode) internal view returns (address) {
         return address(
             uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), CREATE2_DEPLOYER, salt, keccak256(creationCode)))))
         );