fix(IdRegistry): added contract validation to setStorageRegistry

src/IdGateway.sol

@@ -9,6 +9,7 @@ import {TransferHelper} from "./libraries/TransferHelper.sol";
 import {EIP712} from "./abstract/EIP712.sol";
 import {Nonces} from "./abstract/Nonces.sol";
 import {Signatures} from "./abstract/Signatures.sol";
+import {Address} from "openzeppelin/contracts/utils/Address.sol";
 
 /**
  * @title Farcaster IdGateway
@@ -19,6 +20,7 @@ import {Signatures} from "./abstract/Signatures.sol";
  */
 contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces {
     using TransferHelper for address;
+    using Address for address;
 
     /*//////////////////////////////////////////////////////////////
                               CONSTANTS
@@ -90,7 +92,9 @@ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces {
     /**
      * @inheritdoc IIdGateway
      */
-    function price(uint256 extraStorage) external view returns (uint256) {
+    function price(
+        uint256 extraStorage
+    ) external view returns (uint256) {
         return storageRegistry.price(1 + extraStorage);
     }
 
@@ -101,7 +105,9 @@ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces {
     /**
      * @inheritdoc IIdGateway
      */
-    function register(address recovery) external payable returns (uint256, uint256) {
+    function register(
+        address recovery
+    ) external payable returns (uint256, uint256) {
         return register(recovery, 0);
     }
 
@@ -145,7 +151,12 @@ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces {
     /**
      * @inheritdoc IIdGateway
      */
-    function setStorageRegistry(address _storageRegistry) external onlyOwner {
+    function setStorageRegistry(
+        address _storageRegistry
+    ) external onlyOwner {
+        if (_storageRegistry == address(0)) revert Unauthorized();
+        if (!_storageRegistry.isContract()) revert Unauthorized();
+
         emit SetStorageRegistry(address(storageRegistry), _storageRegistry);
         storageRegistry = IStorageRegistry(_storageRegistry);
     }

test/IdGateway/IdGateway.t.sol

@@ -23,15 +23,15 @@ contract IdGatewayTest is IdGatewayTestSuite {
                               PARAMETERS
     //////////////////////////////////////////////////////////////*/
 
-    function testVersion() public {
+    function testVersion() public view {
         assertEq(idGateway.VERSION(), "2023.11.15");
     }
 
-    function testIdRegistry() public {
+    function testIdRegistry() public view {
         assertEq(address(idGateway.idRegistry()), address(idRegistry));
     }
 
-    function testStorageRegistry() public {
+    function testStorageRegistry() public view {
         assertEq(address(idGateway.storageRegistry()), address(storageRegistry));
     }
 
@@ -365,7 +365,7 @@ contract IdGatewayTest is IdGatewayTestSuite {
         assertEq(idRegistry.recoveryOf(1), address(0));
     }
 
-    function testRegisterTypehash() public {
+    function testRegisterTypehash() public view {
         assertEq(
             idGateway.REGISTER_TYPEHASH(),
             keccak256("Register(address to,address recovery,uint256 nonce,uint256 deadline)")
@@ -457,7 +457,15 @@ contract IdGatewayTest is IdGatewayTestSuite {
                         SET STORAGE REGISTRY
     //////////////////////////////////////////////////////////////*/
 
-    function testFuzzSetStorageRegistry(address storageRegistry) public {
+    function testFuzzSetStorageRegistry(
+        address storageRegistry
+    ) public {
+        vm.assume(storageRegistry != address(0));
+        vm.assume(uint160(storageRegistry) > 9);
+
+        // Mocks that the address is a contract
+        vm.etch(storageRegistry, hex"00");
+
         address prevStorageRegistry = address(idGateway.storageRegistry());
 
         vm.expectEmit();

test/IdGateway/IdGatewayTestSuite.sol

@@ -16,11 +16,7 @@ abstract contract IdGatewayTestSuite is StorageRegistryTestSuite, KeyRegistryTes
     function setUp() public virtual override(StorageRegistryTestSuite, KeyRegistryTestSuite) {
         super.setUp();
 
-        idGateway = new IdGateway(
-            address(idRegistry),
-            address(storageRegistry),
-            owner
-        );
+        idGateway = new IdGateway(address(idRegistry), address(storageRegistry), owner);
 
         vm.startPrank(owner);
         idRegistry.setIdGateway(address(idGateway));
@@ -29,7 +25,9 @@ abstract contract IdGatewayTestSuite is StorageRegistryTestSuite, KeyRegistryTes
         addKnownContract(address(idGateway));
     }
 
-    function _registerTo(address caller) internal returns (uint256 fid) {
+    function _registerTo(
+        address caller
+    ) internal returns (uint256 fid) {
         fid = _registerWithRecovery(caller, address(0));
     }
 
@@ -58,7 +56,7 @@ abstract contract IdGatewayTestSuite is StorageRegistryTestSuite, KeyRegistryTes
         address to,
         address recovery,
         uint256 deadline
-    ) internal returns (bytes memory signature) {
+    ) internal view returns (bytes memory signature) {
         address signer = vm.addr(pk);
         bytes32 digest = idGateway.hashTypedDataV4(
             keccak256(abi.encode(idGateway.REGISTER_TYPEHASH(), to, recovery, idGateway.nonces(signer), deadline))

test/IdRegistry/IdRegistryTestSuite.sol

@@ -24,7 +24,9 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup {
                               TEST HELPERS
     //////////////////////////////////////////////////////////////*/
 
-    function _register(address caller) internal returns (uint256 fid) {
+    function _register(
+        address caller
+    ) internal returns (uint256 fid) {
         fid = _registerWithRecovery(caller, address(0));
     }
 
@@ -44,7 +46,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup {
         uint256 fid,
         address to,
         uint256 deadline
-    ) internal returns (bytes memory signature) {
+    ) internal view returns (bytes memory signature) {
         address signer = vm.addr(pk);
         bytes32 digest = idRegistry.hashTypedDataV4(
             keccak256(abi.encode(idRegistry.TRANSFER_TYPEHASH(), fid, to, idRegistry.nonces(signer), deadline))
@@ -60,7 +62,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup {
         address to,
         address recovery,
         uint256 deadline
-    ) internal returns (bytes memory signature) {
+    ) internal view returns (bytes memory signature) {
         address signer = vm.addr(pk);
         bytes32 digest = idRegistry.hashTypedDataV4(
             keccak256(
@@ -85,7 +87,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup {
         address from,
         address to,
         uint256 deadline
-    ) internal returns (bytes memory signature) {
+    ) internal view returns (bytes memory signature) {
         address signer = vm.addr(pk);
         bytes32 digest = idRegistry.hashTypedDataV4(
             keccak256(
@@ -99,7 +101,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup {
         assertEq(signature.length, 65);
     }
 
-    function _signDigest(uint256 pk, bytes32 digest) internal returns (bytes memory signature) {
+    function _signDigest(uint256 pk, bytes32 digest) internal pure returns (bytes memory signature) {
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, digest);
         signature = abi.encodePacked(r, s, v);
         assertEq(signature.length, 65);

test/KeyRegistry/KeyRegistryTestSuite.sol

@@ -32,7 +32,7 @@ abstract contract KeyRegistryTestSuite is IdRegistryTestSuite {
         address owner,
         bytes memory key,
         uint256 deadline
-    ) internal returns (bytes memory signature) {
+    ) internal view returns (bytes memory signature) {
         bytes32 digest = keyRegistry.hashTypedDataV4(
             keccak256(
                 abi.encode(keyRegistry.REMOVE_TYPEHASH(), owner, keccak256(key), keyRegistry.nonces(owner), deadline)

test/TestSuiteSetup.sol

@@ -49,38 +49,44 @@ abstract contract TestSuiteSetup is Test {
                                  HELPERS
     //////////////////////////////////////////////////////////////*/
 
-    function addKnownContract(address contractAddress) public {
+    function addKnownContract(
+        address contractAddress
+    ) public {
         isKnownContract[contractAddress] = true;
     }
 
     // Ensures that a fuzzed address input does not match a known contract address
-    function _assumeClean(address a) internal {
-        assumeNoPrecompiles(a);
+    function _assumeClean(
+        address a
+    ) internal view {
+        assumeNotPrecompile(a);
         vm.assume(!isKnownContract[a]);
         vm.assume(a != ADMIN);
         vm.assume(a != address(0));
     }
 
-    function _boundPk(uint256 pk) internal view returns (uint256) {
+    function _boundPk(
+        uint256 pk
+    ) internal pure returns (uint256) {
         return bound(pk, 1, SECP_256K1_ORDER - 1);
     }
 
-    function _boundDeadline(uint40 deadline) internal view returns (uint256) {
+    function _boundDeadline(
+        uint40 deadline
+    ) internal view returns (uint256) {
         return block.timestamp + uint256(bound(deadline, 0, type(uint40).max));
     }
 
-    function _createMockERC1271(address ownerAddress)
-        internal
-        returns (ERC1271WalletMock mockWallet, address mockWalletAddress)
-    {
+    function _createMockERC1271(
+        address ownerAddress
+    ) internal returns (ERC1271WalletMock mockWallet, address mockWalletAddress) {
         mockWallet = new ERC1271WalletMock(ownerAddress);
         mockWalletAddress = address(mockWallet);
     }
 
-    function _createMaliciousMockERC1271(address ownerAddress)
-        internal
-        returns (ERC1271MaliciousMockForceRevert mockWallet, address mockWalletAddress)
-    {
+    function _createMaliciousMockERC1271(
+        address ownerAddress
+    ) internal returns (ERC1271MaliciousMockForceRevert mockWallet, address mockWalletAddress) {
         mockWallet = new ERC1271MaliciousMockForceRevert(ownerAddress);
         mockWalletAddress = address(mockWallet);
     }