From bc71aba964b170caa29e8afe38c5d6088fe9669a Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 5 Apr 2023 20:33:52 +0100 Subject: [PATCH 1/4] feat: safe guard proof of concept --- contracts/multisigs/SafeGuard.sol | 90 +++++++++++++++++++++++++++++++ test/GnosisSafeMultisig.js | 58 ++++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 contracts/multisigs/SafeGuard.sol diff --git a/contracts/multisigs/SafeGuard.sol b/contracts/multisigs/SafeGuard.sol new file mode 100644 index 00000000..bd67f9c8 --- /dev/null +++ b/contracts/multisigs/SafeGuard.sol @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import "hardhat/console.sol"; + +/// @dev Only `owner` has a privilege, but the `sender` was provided. +/// @param sender Sender address. +/// @param owner Required sender address as an owner. +error OwnerOnly(address sender, address owner); + +/// @dev Provided zero address. +error ZeroAddress(); + +/// @dev Guarded condition is detected. +error Guarded(); + +/// @title SafeGuard - Smart contract for Gnosis Safe Guard functionality +/// @author Aleksandr Kuperman - +contract SafeGuard { + event OwnerUpdated(address indexed owner); + event ChangedGuard(address guard); + // keccak256("guard_manager.guard.address") + bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8; + + // Safe enum + enum Operation {Call, DelegateCall} + + // Contract owner + address public owner; + + /// @dev Guard constructor. + /// @param _owner Guard owner address. + constructor (address _owner) { + owner = _owner; + } + + /// @dev Changes the owner address. + /// @param newOwner Address of a new owner. + function changeOwner(address newOwner) external virtual { + // Check for the ownership + if (msg.sender != owner) { + revert OwnerOnly(msg.sender, owner); + } + + // Check for the zero address + if (newOwner == address(0)) { + revert ZeroAddress(); + } + + owner = newOwner; + emit OwnerUpdated(newOwner); + } + + /// @dev Check transaction function implementation before the Safe's execute function call. + function checkTransaction( + address to, + uint256 value, + bytes memory data, + Operation operation, + uint256 safeTxGas, + uint256 baseGas, + uint256 gasPrice, + address gasToken, + address payable refundReceiver, + bytes memory signatures, + address msgSender + ) external { + // Right now we are blocking all the ETH funds transfer + if (value > 0) { + revert Guarded(); + } + } + + /// @dev Check transaction function implementation after the Safe's execute function call. + function checkAfterExecution(bytes32 txHash, bool success) external { + + } + + /// @dev Sets a guard that checks transactions before and execution. + /// @notice This function copies the corresponding Safe function such that it is correctly initialized. + /// @param guard The address of the guard to be used or the 0 address to disable the guard. + function setGuardForSafe(address guard) external { + bytes32 slot = GUARD_STORAGE_SLOT; + // solhint-disable-next-line no-inline-assembly + assembly { + sstore(slot, guard) + } + emit ChangedGuard(guard); + } +} \ No newline at end of file diff --git a/test/GnosisSafeMultisig.js b/test/GnosisSafeMultisig.js index 473ff3ed..2d41acfc 100644 --- a/test/GnosisSafeMultisig.js +++ b/test/GnosisSafeMultisig.js @@ -132,5 +132,63 @@ describe("GnosisSafeMultisig", function () { } expect(await multisig.getThreshold()).to.equal(threshold); }); + + it("Setting a guard address", async function () { + // Set up a guard contract with the last Safe owner being the guard owner + const SafeGuard = await ethers.getContractFactory("SafeGuard"); + const safeGuard = await SafeGuard.deploy(defaultOwnerAddresses[2]); + await safeGuard.deployed(); + + const to = safeGuard.address; + const fallbackHandler = AddressZero; + const paymentToken = AddressZero; + const paymentReceiver = AddressZero; + const payment = 0; + let nonce = 0; + const payload = safeGuard.interface.encodeFunctionData("setGuardForSafe", [safeGuard.address]); + // Pack the data + const data = ethers.utils.solidityPack(["address", "address", "address", "address", "uint256", "uint256", "bytes"], + [to, fallbackHandler, paymentToken, paymentReceiver, payment, nonce, payload]); + + // Create a multisig with the packed data + const tx = await gnosisSafeMultisig.create(defaultOwnerAddresses, threshold, data); + const result = await tx.wait(); + + // Check that the obtained address is not zero + expect(result.events[0].address).to.not.equal(AddressZero); + const proxyAddress = result.events[0].address; + + // Get the safe multisig instance + const multisig = await ethers.getContractAt("GnosisSafe", proxyAddress); + + // Check the multisig owners and threshold + const owners = await multisig.getOwners(); + for (let i = 0; i < defaultOwnerAddresses.length; i++) { + expect(owners[i]).to.equal(defaultOwnerAddresses[i]); + } + expect(await multisig.getThreshold()).to.equal(threshold); + + // Check the guard slot + const guardSlot = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8"; + let slotValue = await ethers.provider.getStorageAt(multisig.address, guardSlot); + slotValue = ethers.utils.hexlify(ethers.utils.stripZeros(slotValue)); + const safeGuardAddress = safeGuard.address.toLowerCase(); + expect(slotValue).to.equal(safeGuardAddress); + + // Send funds to the multisig address + await signers[0].sendTransaction({to: multisig.address, value: ethers.utils.parseEther("1000")}); + + // Try to send ETH funds from the multisig using first two Safe owners while facing the guard check + const safeContracts = require("@gnosis.pm/safe-contracts"); + nonce = await multisig.nonce(); + // The function call is irrelevant, we just need to pass the value + let txHashData = await safeContracts.buildContractCall(multisig, "nonce", [], nonce, 0, 0); + txHashData.value = ethers.utils.parseEther("10"); + const signMessageData = [await safeContracts.safeSignMessage(signers[1], multisig, txHashData, 0), + await safeContracts.safeSignMessage(signers[2], multisig, txHashData, 0)]; + await expect( + safeContracts.executeTx(multisig, txHashData, signMessageData, 0) + ).to.be.revertedWith("Guarded"); + }); }); }); From 823c5db2b6286159b57abfb4b27c69e8521e90a8 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 5 Apr 2023 20:36:10 +0100 Subject: [PATCH 2/4] chore: removing console --- contracts/multisigs/SafeGuard.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/multisigs/SafeGuard.sol b/contracts/multisigs/SafeGuard.sol index bd67f9c8..7dfffd9d 100644 --- a/contracts/multisigs/SafeGuard.sol +++ b/contracts/multisigs/SafeGuard.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.19; -import "hardhat/console.sol"; - /// @dev Only `owner` has a privilege, but the `sender` was provided. /// @param sender Sender address. /// @param owner Required sender address as an owner. From 6b40877199e4f67aa7e9221322beb81390215ed6 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 5 Apr 2023 20:43:10 +0100 Subject: [PATCH 3/4] chore: updating the test name --- test/GnosisSafeMultisig.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/GnosisSafeMultisig.js b/test/GnosisSafeMultisig.js index 2d41acfc..1913bc6c 100644 --- a/test/GnosisSafeMultisig.js +++ b/test/GnosisSafeMultisig.js @@ -133,7 +133,7 @@ describe("GnosisSafeMultisig", function () { expect(await multisig.getThreshold()).to.equal(threshold); }); - it("Setting a guard address", async function () { + it("Setting a guard contract address that guards transactions for not being able to transfer ETH funds", async function () { // Set up a guard contract with the last Safe owner being the guard owner const SafeGuard = await ethers.getContractFactory("SafeGuard"); const safeGuard = await SafeGuard.deploy(defaultOwnerAddresses[2]); From f9e92e71e7668ee0246e8f3e71fdcccedb9b5ced Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Wed, 5 Apr 2023 20:44:24 +0100 Subject: [PATCH 4/4] chore: correcting variable name in the test for a better understanding --- test/GnosisSafeMultisig.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/GnosisSafeMultisig.js b/test/GnosisSafeMultisig.js index 1913bc6c..6d87eeb4 100644 --- a/test/GnosisSafeMultisig.js +++ b/test/GnosisSafeMultisig.js @@ -136,7 +136,7 @@ describe("GnosisSafeMultisig", function () { it("Setting a guard contract address that guards transactions for not being able to transfer ETH funds", async function () { // Set up a guard contract with the last Safe owner being the guard owner const SafeGuard = await ethers.getContractFactory("SafeGuard"); - const safeGuard = await SafeGuard.deploy(defaultOwnerAddresses[2]); + const safeGuard = await SafeGuard.deploy(signers[2].address); await safeGuard.deployed(); const to = safeGuard.address;