Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: safe guard proof of concept #68

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions contracts/multisigs/SafeGuard.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

/// @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 - <aleksandr.kuperman@valory.xyz>
contract SafeGuard {
event OwnerUpdated(address indexed owner);
event ChangedGuard(address guard);
// keccak256("guard_manager.guard.address")
bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the Safe guard slot that will be used in order to set a guard


// 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();
}
}
Comment on lines +52 to +70
Copy link
Collaborator Author

@kupermind kupermind Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementations of the checkTransaction() function that will be called by a guard before the Safe execute() function call. For the exercise, this function blocks all the calls having value parameter being greater than zero.


/// @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);
}
Comment on lines +77 to +87
Copy link
Collaborator Author

@kupermind kupermind Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is called via a delegatecall from the Safe module setup when Safe is created. Since the delegatecall glues the storage space of the Safe instance being created and the SafeGuard contract, setting the guard slot would set it in the Safe instance.

}
58 changes: 58 additions & 0 deletions test/GnosisSafeMultisig.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,63 @@ describe("GnosisSafeMultisig", function () {
}
expect(await multisig.getThreshold()).to.equal(threshold);
});

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(signers[2].address);
await safeGuard.deployed();

const to = safeGuard.address;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe Guard address is the guard contract that is going to be called in the Safe modules setup, and is able to set the guard address slot value as it is called via a delegatecall() from the created Safe instance.

const fallbackHandler = AddressZero;
const paymentToken = AddressZero;
const paymentReceiver = AddressZero;
const payment = 0;
let nonce = 0;
const payload = safeGuard.interface.encodeFunctionData("setGuardForSafe", [safeGuard.address]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The payload is included to be called by the modules setup, calling the Safe Guard contract with the specified function setting the guard 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();
Comment on lines +153 to +155
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the function called from the ServiceRegistry deploy() function.


// 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);
Comment on lines +171 to +176
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking that the create Safe instance has the correctly set guard slot with the required address.


// 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");
Comment on lines +181 to +191
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This set of actions simulates the Safe instance call execution that tries to transfer some value, and it is blocked by the Safe Guard.

});
});
});