diff --git a/contracts/multisigs/GnosisSafeSameAddressMultisig.sol b/contracts/multisigs/GnosisSafeSameAddressMultisig.sol index b4038aa1..6c0fedf0 100644 --- a/contracts/multisigs/GnosisSafeSameAddressMultisig.sol +++ b/contracts/multisigs/GnosisSafeSameAddressMultisig.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.15; +pragma solidity ^0.8.21; // Gnosis Safe Master Copy interface extracted from the mainnet: https://etherscan.io/address/0xd9db270c1b5e3bd161e8c8503c55ceabee709552#code#F6#L126 interface IGnosisSafe { @@ -12,6 +12,16 @@ interface IGnosisSafe { function getThreshold() external view returns (uint256); } +/// @dev Zero value when it has to be different from zero. +error ZeroValue(); + +/// @dev Provided zero address. +error ZeroAddress(); + +/// @dev Multisig proxy bytecode is not whitelisted. +/// @param multisig Address of a multisig proxy. +error UnauthorizedMultisig(address multisig); + /// @dev Provided incorrect data length. /// @param expected Expected minimum data length. /// @param provided Provided data length. @@ -42,6 +52,31 @@ contract GnosisSafeSameAddressMultisig { // This exact size suggests that all the changes to the multisig have been performed and only validation is needed uint256 public constant DEFAULT_DATA_LENGTH = 20; + // Map of approved multisig proxy hashes + mapping(bytes32 => bool) public mapMultisigHashes; + + /// @dev GnosisSafeSameAddressMultisig constructor. + /// @param _multisigProxyHashes Multisig proxy hashes. + constructor(bytes32[] memory _multisigProxyHashes) { + // There must be at least one multisig proxy hash + uint256 size = _multisigProxyHashes.length; + if (size == 0) { + revert ZeroValue(); + } + + // Record provided multisig proxy bytecode hashes + for (uint256 i = 0; i < size; ++i) { + bytes32 proxyHash = _multisigProxyHashes[i]; + // Check for the zero hash + if (proxyHash == bytes32(0)) { + revert ZeroValue(); + } + + // Hash the proxy bytecode + mapMultisigHashes[proxyHash] = true; + } + } + /// @dev Updates and/or verifies the existent gnosis safe multisig for changed owners and threshold. /// @notice This function operates with existent multisig proxy that is requested to be updated in terms of /// the set of owners' addresses and the threshold. There are two scenarios possible: @@ -75,6 +110,12 @@ contract GnosisSafeSameAddressMultisig { multisig := mload(add(data, DEFAULT_DATA_LENGTH)) } + // Check that the multisig address corresponds to the authorized multisig proxy + bytes32 proxyHash = keccak256(multisig.code); + if (!mapMultisigHashes[proxyHash]) { + revert UnauthorizedMultisig(multisig); + } + // If provided, read the payload that is going to change the multisig ownership and threshold // The payload is expected to be the `execTransaction()` function call with all its arguments and signature(s) if (dataLength > DEFAULT_DATA_LENGTH) { diff --git a/test/GnosisSafeSameAddressMultisig.js b/test/GnosisSafeSameAddressMultisig.js index 408aa0e5..482951bd 100644 --- a/test/GnosisSafeSameAddressMultisig.js +++ b/test/GnosisSafeSameAddressMultisig.js @@ -15,6 +15,7 @@ describe("GnosisSafeSameAddressMultisig", function () { const newThreshold = 3; const AddressZero = "0x" + "0".repeat(40); const maxUint256 = "115792089237316195423570985008687907853269984665640564039457584007913129639935"; + let bytecodeHash; beforeEach(async function () { const GnosisSafe = await ethers.getContractFactory("GnosisSafe"); @@ -29,8 +30,14 @@ describe("GnosisSafeSameAddressMultisig", function () { multiSend = await MultiSend.deploy(); await multiSend.deployed(); + const GnosisSafeProxy = await ethers.getContractFactory("GnosisSafeProxy"); + const gnosisSafeProxy = await GnosisSafeProxy.deploy(gnosisSafe.address); + await gnosisSafeProxy.deployed(); + const bytecode = await ethers.provider.getCode(gnosisSafeProxy.address); + bytecodeHash = ethers.utils.keccak256(bytecode); + const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig"); - gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy(); + gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]); await gnosisSafeSameAddressMultisig.deployed(); signers = await ethers.getSigners(); @@ -39,6 +46,19 @@ describe("GnosisSafeSameAddressMultisig", function () { }); context("Verifying multisigs", async function () { + it("Try to deploy a contract without specified proxies or zero hashes", async function () { + const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig"); + + await expect( + GnosisSafeSameAddressMultisig.deploy([]) + ).to.be.revertedWithCustomError(gnosisSafeSameAddressMultisig, "ZeroValue"); + + const bytes32Zero = "0x" + "0".repeat(64); + await expect( + GnosisSafeSameAddressMultisig.deploy([bytes32Zero]) + ).to.be.revertedWithCustomError(gnosisSafeSameAddressMultisig, "ZeroValue"); + }); + it("Should fail when passing the non-zero multisig data with the incorrect number of bytes", async function () { await expect( gnosisSafeSameAddressMultisig.create(newOwnerAddresses, initialThreshold, "0x55") diff --git a/test/ServiceManagementWithOperatorSignatures.js b/test/ServiceManagementWithOperatorSignatures.js index 78d94c56..9d963a7e 100644 --- a/test/ServiceManagementWithOperatorSignatures.js +++ b/test/ServiceManagementWithOperatorSignatures.js @@ -30,6 +30,7 @@ describe("ServiceManagementWithOperatorSignatures", function () { const AddressZero = "0x" + "0".repeat(40); const ETHAddress = "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE"; const payload = "0x"; + let bytecodeHash; beforeEach(async function () { const ComponentRegistry = await ethers.getContractFactory("ComponentRegistry"); @@ -62,8 +63,14 @@ describe("ServiceManagementWithOperatorSignatures", function () { multiSend = await MultiSend.deploy(); await multiSend.deployed(); + const GnosisSafeProxy = await ethers.getContractFactory("GnosisSafeProxy"); + const gnosisSafeProxy = await GnosisSafeProxy.deploy(gnosisSafe.address); + await gnosisSafeProxy.deployed(); + const bytecode = await ethers.provider.getCode(gnosisSafeProxy.address); + bytecodeHash = ethers.utils.keccak256(bytecode); + const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig"); - gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy(); + gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]); await gnosisSafeSameAddressMultisig.deployed(); const ServiceRegistry = await ethers.getContractFactory("ServiceRegistry"); diff --git a/test/ServiceRegistry.js b/test/ServiceRegistry.js index e455edc4..f1603afc 100644 --- a/test/ServiceRegistry.js +++ b/test/ServiceRegistry.js @@ -40,6 +40,7 @@ describe("ServiceRegistry", function () { const AddressZero = "0x" + "0".repeat(40); const payload = "0x"; const serviceRegistryImplementations = ["l1", "l2", "l1an"]; + let bytecodeHash; beforeEach(async function () { const ComponentRegistry = await ethers.getContractFactory("ComponentRegistry"); @@ -72,8 +73,14 @@ describe("ServiceRegistry", function () { multiSend = await MultiSend.deploy(); await multiSend.deployed(); + const GnosisSafeProxy = await ethers.getContractFactory("GnosisSafeProxy"); + const gnosisSafeProxy = await GnosisSafeProxy.deploy(gnosisSafe.address); + await gnosisSafeProxy.deployed(); + const bytecode = await ethers.provider.getCode(gnosisSafeProxy.address); + bytecodeHash = ethers.utils.keccak256(bytecode); + const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig"); - gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy(); + gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]); await gnosisSafeSameAddressMultisig.deployed(); const ServiceRegistry = await ethers.getContractFactory("ServiceRegistry"); diff --git a/test/ServiceStaking.js b/test/ServiceStaking.js index e4d58af0..ca74b615 100644 --- a/test/ServiceStaking.js +++ b/test/ServiceStaking.js @@ -93,7 +93,7 @@ describe("ServiceStakingNativeToken", function () { bytecodeHash = ethers.utils.keccak256(bytecode); const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig"); - gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy(); + gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]); await gnosisSafeSameAddressMultisig.deployed(); const MultiSend = await ethers.getContractFactory("MultiSendCallOnly"); @@ -1025,15 +1025,9 @@ describe("ServiceStakingNativeToken", function () { // Prepare the payload to redeploy with the attacker address const data = ethers.utils.solidityPack(["address"], [attacker.address]); - await serviceRegistry.deploy(deployer.address, serviceId, gnosisSafeSameAddressMultisig.address, data); - - // Approve service - await serviceRegistry.approve(serviceStaking.address, serviceId); - - // Try to stake the service await expect( - serviceStaking.stake(serviceId) - ).to.be.revertedWithCustomError(serviceStaking, "UnauthorizedMultisig"); + serviceRegistry.deploy(deployer.address, serviceId, gnosisSafeSameAddressMultisig.address, data) + ).to.be.revertedWithCustomError(gnosisSafeSameAddressMultisig, "UnauthorizedMultisig"); // Restore a previous state of blockchain snapshot.restore(); diff --git a/test/ServiceStaking.t.sol b/test/ServiceStaking.t.sol index 36913c85..44db916b 100644 --- a/test/ServiceStaking.t.sol +++ b/test/ServiceStaking.t.sol @@ -12,7 +12,6 @@ import {ServiceRegistryTokenUtility} from "../contracts/ServiceRegistryTokenUtil import {ServiceManagerToken} from "../contracts/ServiceManagerToken.sol"; import {OperatorWhitelist} from "../contracts/utils/OperatorWhitelist.sol"; import {GnosisSafeMultisig} from "../contracts/multisigs/GnosisSafeMultisig.sol"; -import {GnosisSafeSameAddressMultisig} from "../contracts/multisigs/GnosisSafeSameAddressMultisig.sol"; import "../contracts/staking/ServiceStakingNativeToken.sol"; import {ServiceStakingToken} from "../contracts/staking/ServiceStakingToken.sol"; import {SafeNonceLib} from "../contracts/test/SafeNonceLib.sol"; @@ -28,7 +27,6 @@ contract BaseSetup is Test { GnosisSafeProxy internal gnosisSafeProxy; GnosisSafeProxyFactory internal gnosisSafeProxyFactory; GnosisSafeMultisig internal gnosisSafeMultisig; - GnosisSafeSameAddressMultisig internal gnosisSafeSameAddressMultisig; ServiceStakingNativeToken internal serviceStakingNativeToken; ServiceStakingToken internal serviceStakingToken; SafeNonceLib internal safeNonceLib; @@ -93,7 +91,6 @@ contract BaseSetup is Test { gnosisSafeProxy = new GnosisSafeProxy(address(gnosisSafe)); gnosisSafeProxyFactory = new GnosisSafeProxyFactory(); gnosisSafeMultisig = new GnosisSafeMultisig(payable(address(gnosisSafe)), address(gnosisSafeProxyFactory)); - gnosisSafeSameAddressMultisig = new GnosisSafeSameAddressMultisig(); safeNonceLib = new SafeNonceLib(); // Deploying a token contract and minting to deployer, operator and a current contract @@ -102,19 +99,20 @@ contract BaseSetup is Test { token.mint(operator, initialMint); token.mint(address(this), initialMint); - // Deploy service staking native token and arbitraty token + // Get the multisig proxy bytecode hash + bytes32[] memory multisigProxyHashes = new bytes32[](1); + multisigProxyHashes[0] = keccak256(address(gnosisSafeProxy).code); + + // Deploy service staking native token and arbitrary ERC20 token ServiceStakingBase.StakingParams memory stakingParams = ServiceStakingBase.StakingParams(maxNumServices, rewardsPerSecond, minStakingDeposit, livenessPeriod, livenessRatio, numAgentInstances, emptyArray, 0, bytes32(0)); - bytes32[] memory multisigProxyAddresses = new bytes32[](1); - multisigProxyAddresses[0] = keccak256(address(gnosisSafeProxy).code); serviceStakingNativeToken = new ServiceStakingNativeToken(stakingParams, address(serviceRegistry), - multisigProxyAddresses); + multisigProxyHashes); serviceStakingToken = new ServiceStakingToken(stakingParams, address(serviceRegistry), address(serviceRegistryTokenUtility), - address(token), multisigProxyAddresses); + address(token), multisigProxyHashes); // Whitelist multisig implementations serviceRegistry.changeMultisigPermission(address(gnosisSafeMultisig), true); - serviceRegistry.changeMultisigPermission(address(gnosisSafeSameAddressMultisig), true); IService.AgentParams[] memory agentParams = new IService.AgentParams[](1); agentParams[0].slots = 1;