Skip to content

Commit ad6c735

Browse files
authored
Post tx guard (#304)
1 parent 9aee4c4 commit ad6c735

16 files changed

+539
-103
lines changed

contracts/GnosisSafe.sol

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,6 @@ contract GnosisSafe is
4242
// );
4343
bytes32 private constant SAFE_TX_TYPEHASH = 0xbb8310d486368db6bd6f849402fdd73ad53d316b5a4b2644ad6efe0f941286d8;
4444

45-
//keccak256(
46-
// "SafeMessage(bytes message)"
47-
//);
48-
bytes32 private constant SAFE_MSG_TYPEHASH = 0x60b3cbf8b4a223d68d641b3b6ddf9a298e7f33710cf3d3a9d1146b5a6150fbca;
49-
5045
event SafeSetup(address indexed initiator, address[] owners, uint256 threshold, address initializer, address fallbackHandler);
5146
event ApproveHash(bytes32 indexed approvedHash, address indexed owner);
5247
event SignMsg(bytes32 indexed msgHash);
@@ -149,8 +144,8 @@ contract GnosisSafe is
149144
txHash = keccak256(txHashData);
150145
checkSignatures(txHash, txHashData, signatures);
151146
}
147+
address guard = getGuard();
152148
{
153-
address guard = getGuard();
154149
if (guard != address(0)) {
155150
Guard(guard).checkTransaction(
156151
// Transaction info
@@ -191,6 +186,11 @@ contract GnosisSafe is
191186
if (success) emit ExecutionSuccess(txHash, payment);
192187
else emit ExecutionFailure(txHash, payment);
193188
}
189+
{
190+
if (guard != address(0)) {
191+
Guard(guard).checkAfterExecution(txHash, success);
192+
}
193+
}
194194
}
195195

196196
function handlePayment(
@@ -336,17 +336,6 @@ contract GnosisSafe is
336336
emit ApproveHash(hashToApprove, msg.sender);
337337
}
338338

339-
/**
340-
* @dev Marks a message as signed, so that it can be used with EIP-1271
341-
* @notice Marks a message (`_data`) as signed.
342-
* @param _data Arbitrary length data that should be marked as signed on the behalf of address(this)
343-
*/
344-
function signMessage(bytes calldata _data) external authorized {
345-
bytes32 msgHash = getMessageHash(_data);
346-
signedMessages[msgHash] = 1;
347-
emit SignMsg(msgHash);
348-
}
349-
350339
/// @dev Returns the chain id used by this contract.
351340
function getChainId() public view returns (uint256) {
352341
uint256 id;
@@ -361,14 +350,6 @@ contract GnosisSafe is
361350
return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, getChainId(), this));
362351
}
363352

364-
/// @dev Returns hash of a message that can be signed by owners.
365-
/// @param message Message that should be hashed
366-
/// @return Message hash.
367-
function getMessageHash(bytes memory message) public view returns (bytes32) {
368-
bytes32 safeMessageHash = keccak256(abi.encode(SAFE_MSG_TYPEHASH, keccak256(message)));
369-
return keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeMessageHash));
370-
}
371-
372353
/// @dev Returns the bytes that are hashed to be signed by owners.
373354
/// @param to Destination address.
374355
/// @param value Ether value.

contracts/base/GuardManager.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ interface Guard {
1818
bytes memory signatures,
1919
address msgSender
2020
) external;
21+
22+
function checkAfterExecution(bytes32 txHash, bool success) external;
2123
}
2224

2325
/// @title Fallback Manager - A contract that manages fallback calls made to this contract
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// SPDX-License-Identifier: LGPL-3.0-only
2+
pragma solidity >=0.7.0 <0.9.0;
3+
4+
import "../../common/Enum.sol";
5+
import "../../base/GuardManager.sol";
6+
import "../../GnosisSafe.sol";
7+
8+
/// @title Debug Transaction Guard - A guard that will emit events with extended information.
9+
/// @notice This guard is only meant as a development tool and example
10+
/// @author Richard Meissner - <richard@gnosis.pm>
11+
contract DebugTransactionGuard is Guard {
12+
// solhint-disable-next-line payable-fallback
13+
fallback() external {
14+
// We don't revert on fallback to avoid issues in case of a Safe upgrade
15+
// E.g. The expected check method might change and then the Safe would be locked.
16+
}
17+
18+
event TransactionDetails(
19+
address indexed safe,
20+
bytes32 indexed txHash,
21+
address to,
22+
uint256 value,
23+
bytes data,
24+
Enum.Operation operation,
25+
uint256 safeTxGas,
26+
bool usesRefund,
27+
uint256 nonce
28+
);
29+
30+
event GasUsage(address indexed safe, bytes32 indexed txHash, uint256 indexed nonce, bool success);
31+
32+
mapping(bytes32 => uint256) public txNonces;
33+
34+
function checkTransaction(
35+
address to,
36+
uint256 value,
37+
bytes memory data,
38+
Enum.Operation operation,
39+
uint256 safeTxGas,
40+
uint256 baseGas,
41+
uint256 gasPrice,
42+
address gasToken,
43+
// solhint-disable-next-line no-unused-vars
44+
address payable refundReceiver,
45+
bytes memory,
46+
address
47+
) external override {
48+
uint256 nonce;
49+
bytes32 txHash;
50+
{
51+
GnosisSafe safe = GnosisSafe(payable(msg.sender));
52+
nonce = safe.nonce() - 1;
53+
txHash = safe.getTransactionHash(to, value, data, operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, nonce);
54+
}
55+
emit TransactionDetails(msg.sender, txHash, to, value, data, operation, safeTxGas, gasPrice > 0, nonce);
56+
txNonces[txHash] = nonce;
57+
}
58+
59+
function checkAfterExecution(bytes32 txHash, bool success) external override {
60+
uint256 nonce = txNonces[txHash];
61+
require(nonce != 0, "Could not get nonce");
62+
txNonces[txHash] = 0;
63+
emit GasUsage(msg.sender, txHash, nonce, success);
64+
}
65+
}

contracts/guards/DelegateCallTransactionGuard.sol renamed to contracts/examples/guards/DelegateCallTransactionGuard.sol

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// SPDX-License-Identifier: LGPL-3.0-only
22
pragma solidity >=0.7.0 <0.9.0;
33

4-
import "../common/Enum.sol";
5-
import "../base/GuardManager.sol";
6-
import "../GnosisSafe.sol";
4+
import "../../common/Enum.sol";
5+
import "../../base/GuardManager.sol";
6+
import "../../GnosisSafe.sol";
77

88
contract DelegateCallTransactionGuard is Guard {
99
address public immutable allowedTarget;
@@ -34,4 +34,6 @@ contract DelegateCallTransactionGuard is Guard {
3434
) external view override {
3535
require(operation != Enum.Operation.DelegateCall || to == allowedTarget, "This call is restricted");
3636
}
37+
38+
function checkAfterExecution(bytes32, bool) external view override {}
3739
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// SPDX-License-Identifier: LGPL-3.0-only
2+
pragma solidity >=0.7.0 <0.9.0;
3+
4+
import "../../common/Enum.sol";
5+
import "../../base/GuardManager.sol";
6+
import "../../GnosisSafe.sol";
7+
8+
contract ReentrencyTransactionGuard is Guard {
9+
bytes32 internal constant GUARD_STORAGE_SLOT = keccak256("reentrentry_guard.guard.struct");
10+
11+
struct GuardValue {
12+
bool active;
13+
}
14+
15+
// solhint-disable-next-line payable-fallback
16+
fallback() external {
17+
// We don't revert on fallback to avoid issues in case of a Safe upgrade
18+
// E.g. The expected check method might change and then the Safe would be locked.
19+
}
20+
21+
function getGuard() internal pure returns (GuardValue storage guard) {
22+
bytes32 slot = GUARD_STORAGE_SLOT;
23+
// solhint-disable-next-line no-inline-assembly
24+
assembly {
25+
guard.slot := slot
26+
}
27+
}
28+
29+
function checkTransaction(
30+
address,
31+
uint256,
32+
bytes memory,
33+
Enum.Operation,
34+
uint256,
35+
uint256,
36+
uint256,
37+
address,
38+
// solhint-disable-next-line no-unused-vars
39+
address payable,
40+
bytes memory,
41+
address
42+
) external override {
43+
GuardValue storage guard = getGuard();
44+
require(!guard.active, "Reentrency detected");
45+
guard.active = true;
46+
}
47+
48+
function checkAfterExecution(bytes32, bool) external override {
49+
getGuard().active = false;
50+
}
51+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// SPDX-License-Identifier: LGPL-3.0-only
2+
pragma solidity >=0.7.0 <0.9.0;
3+
4+
/// @title GnosisSafeStorage - Storage layout of the Safe contracts to be used in libraries
5+
/// @author Richard Meissner - <richard@gnosis.io>
6+
contract GnosisSafeStorage {
7+
// From /common/Singleton.sol
8+
address internal singleton;
9+
// From /common/ModuleManager.sol
10+
mapping(address => address) internal modules;
11+
// From /common/OwnerManager.sol
12+
mapping(address => address) internal owners;
13+
uint256 internal ownerCount;
14+
uint256 internal threshold;
15+
16+
// From /GnosisSafe.sol
17+
bytes32 internal nonce;
18+
bytes32 internal domainSeparator;
19+
mapping(bytes32 => uint256) internal signedMessages;
20+
mapping(address => mapping(bytes32 => uint256)) internal approvedHashes;
21+
}

contracts/libraries/Migrate_1_3_0_to_1_2_0.sol renamed to contracts/examples/libraries/Migrate_1_3_0_to_1_2_0.sol

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
// SPDX-License-Identifier: LGPL-3.0-only
22
pragma solidity >=0.7.0 <0.9.0;
3+
import "./GnosisSafeStorage.sol";
34

45
/// @title Migration - migrates a Safe contract from 1.3.0 to 1.2.0
56
/// @author Richard Meissner - <richard@gnosis.io>
6-
contract Migration {
7+
contract Migration is GnosisSafeStorage {
78
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = 0x035aff83d86937d35b32e04f0ddc6ff469290eef2f1b692d8a815c89404d4749;
89

910
address public immutable migrationSingleton;
@@ -17,18 +18,6 @@ contract Migration {
1718

1819
event ChangedMasterCopy(address singleton);
1920

20-
// Here to have the same storage layout as the Safe
21-
address private singleton;
22-
// Some unused slots to have the correct layout
23-
bytes32 private _slot1;
24-
bytes32 private _slot2;
25-
bytes32 private _slot3;
26-
bytes32 private _slot4;
27-
bytes32 private _slot5;
28-
// For the migration we need to set the old domain separator in storage
29-
bytes32 private domainSeparator;
30-
31-
// Define guard last to avoid conflict with Safe storage layout
3221
bytes32 private guard;
3322

3423
/// @dev Allows to migrate the contract. This can only be called via a delegatecall.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// SPDX-License-Identifier: LGPL-3.0-only
2+
pragma solidity >=0.7.0 <0.9.0;
3+
4+
import "./GnosisSafeStorage.sol";
5+
import "../../GnosisSafe.sol";
6+
7+
/// @title SignMessageLib - Allows to set an entry in the signedMessages
8+
/// @author Richard Meissner - <richard@gnosis.io>
9+
contract SignMessageLib is GnosisSafeStorage {
10+
//keccak256(
11+
// "SafeMessage(bytes message)"
12+
//);
13+
bytes32 private constant SAFE_MSG_TYPEHASH = 0x60b3cbf8b4a223d68d641b3b6ddf9a298e7f33710cf3d3a9d1146b5a6150fbca;
14+
15+
event SignMsg(bytes32 indexed msgHash);
16+
17+
/// @dev Marks a message as signed, so that it can be used with EIP-1271
18+
/// @notice Marks a message (`_data`) as signed.
19+
/// @param _data Arbitrary length data that should be marked as signed on the behalf of address(this)
20+
function signMessage(bytes calldata _data) external {
21+
bytes32 msgHash = getMessageHash(_data);
22+
signedMessages[msgHash] = 1;
23+
emit SignMsg(msgHash);
24+
}
25+
26+
/// @dev Returns hash of a message that can be signed by owners.
27+
/// @param message Message that should be hashed
28+
/// @return Message hash.
29+
function getMessageHash(bytes memory message) public view returns (bytes32) {
30+
bytes32 safeMessageHash = keccak256(abi.encode(SAFE_MSG_TYPEHASH, keccak256(message)));
31+
return
32+
keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), GnosisSafe(payable(address(this))).domainSeparator(), safeMessageHash));
33+
}
34+
}

contracts/handler/CompatibilityFallbackHandler.sol

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import "../GnosisSafe.sol";
88
/// @title Compatibility Fallback Handler - fallback handler to provider compatibility between pre 1.3.0 and 1.3.0+ Safe contracts
99
/// @author Richard Meissner - <richard@gnosis.pm>
1010
contract CompatibilityFallbackHandler is DefaultCallbackHandler, ISignatureValidator {
11+
//keccak256(
12+
// "SafeMessage(bytes message)"
13+
//);
14+
bytes32 private constant SAFE_MSG_TYPEHASH = 0x60b3cbf8b4a223d68d641b3b6ddf9a298e7f33710cf3d3a9d1146b5a6150fbca;
15+
1116
bytes4 internal constant SIMULATE_SELECTOR = bytes4(keccak256("simulate(address,bytes)"));
1217

1318
address internal constant SENTINEL_MODULES = address(0x1);
@@ -23,7 +28,7 @@ contract CompatibilityFallbackHandler is DefaultCallbackHandler, ISignatureValid
2328
function isValidSignature(bytes calldata _data, bytes calldata _signature) public view override returns (bytes4) {
2429
// Caller should be a Safe
2530
GnosisSafe safe = GnosisSafe(payable(msg.sender));
26-
bytes32 messageHash = safe.getMessageHash(_data);
31+
bytes32 messageHash = getMessageHashForSafe(safe, _data);
2732
if (_signature.length == 0) {
2833
require(safe.signedMessages(messageHash) != 0, "Hash not approved");
2934
} else {
@@ -32,6 +37,22 @@ contract CompatibilityFallbackHandler is DefaultCallbackHandler, ISignatureValid
3237
return EIP1271_MAGIC_VALUE;
3338
}
3439

40+
/// @dev Returns hash of a message that can be signed by owners.
41+
/// @param message Message that should be hashed
42+
/// @return Message hash.
43+
function getMessageHash(bytes memory message) public view returns (bytes32) {
44+
return getMessageHashForSafe(GnosisSafe(payable(msg.sender)), message);
45+
}
46+
47+
/// @dev Returns hash of a message that can be signed by owners.
48+
/// @param safe Safe to which the message is targeted
49+
/// @param message Message that should be hashed
50+
/// @return Message hash.
51+
function getMessageHashForSafe(GnosisSafe safe, bytes memory message) public view returns (bytes32) {
52+
bytes32 safeMessageHash = keccak256(abi.encode(SAFE_MSG_TYPEHASH, keccak256(message)));
53+
return keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), safe.domainSeparator(), safeMessageHash));
54+
}
55+
3556
/**
3657
* Implementation of updated EIP-1271
3758
* @dev Should return whether the signature provided is valid for the provided data.

0 commit comments

Comments
 (0)