Skip to content

Commit 1ba2cad

Browse files
committed
Found an approach that might work
1 parent 36d6e73 commit 1ba2cad

File tree

4 files changed

+246
-36
lines changed

4 files changed

+246
-36
lines changed

contracts/solc-0.8/CPKFactory.sol

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,23 @@ pragma solidity >=0.8.0;
33

44
import { IGnosisSafeProxyFactory } from "./dep-ports/IGnosisSafeProxyFactory.sol";
55
import { ProxyImplSetter } from "./ProxyImplSetter.sol";
6+
import { SafeSignatureUtils } from "./SafeSignatureUtils.sol";
7+
8+
enum TxReaction {
9+
RevertOnReturnFalse,
10+
CaptureBoolReturn,
11+
IgnoreReturn
12+
}
613

714
struct CPKFactoryTx {
815
uint value;
916
bytes data;
17+
TxReaction reaction;
1018
}
1119

1220
contract CPKFactory {
21+
using SafeSignatureUtils for bytes;
22+
1323
event CPKCreation(
1424
address indexed proxy,
1525
address initialImpl,
@@ -40,12 +50,17 @@ contract CPKFactory {
4050
address owner,
4151
address safeVersion,
4252
uint256 salt,
43-
CPKFactoryTx[] calldata txs
53+
CPKFactoryTx[] calldata txs,
54+
bytes calldata signature
4455
)
4556
external
4657
payable
4758
returns (bool execTransactionSuccess)
4859
{
60+
bytes memory data = abi.encode(safeVersion, salt, txs);
61+
bytes32 dataHash = keccak256(data);
62+
signature.check(dataHash, data, owner);
63+
4964
bytes32 saltNonce = keccak256(abi.encode(owner, salt));
5065

5166
address payable proxy = gnosisSafeProxyFactory.createProxyWithNonce(
@@ -57,33 +72,38 @@ contract CPKFactory {
5772
ProxyImplSetter(proxy).setImplementation(safeVersion);
5873

5974
uint sumTxsValues = 0;
60-
bytes memory lastReturnData;
6175
for (uint i = 0; i < txs.length; i++) {
6276
bool txSuccess;
77+
bytes memory returnData;
6378
uint txValue = txs[i].value;
6479
sumTxsValues += txValue;
65-
(txSuccess, lastReturnData) = proxy.call{value: txValue}(txs[i].data);
80+
(txSuccess, returnData) = proxy.call{value: txValue}(txs[i].data);
6681
assembly {
6782
// txSuccess == 0 means the call failed
6883
if iszero(txSuccess) {
69-
// The revert data begins one word after the lastReturnData pointer.
70-
// At the location lastReturnData in memory, the length of the bytes is stored.
71-
// This differs from the high-level revert(string(lastReturnData))
72-
// as the high-level version encodes the lastReturnData in a Error(string) object.
84+
// The revert data begins one word after the returnData pointer.
85+
// At the location returnData in memory, the length of the bytes is stored.
86+
// This differs from the high-level revert(string(returnData))
87+
// as the high-level version encodes the returnData in a Error(string) object.
7388
// We want to avoid that because the underlying call should have already
7489
// formatted the data in an Error(string) object
75-
revert(add(0x20, lastReturnData), mload(lastReturnData))
90+
revert(add(0x20, returnData), mload(returnData))
7691
}
7792
}
93+
94+
TxReaction txReaction = txs[i].reaction;
95+
if (txReaction == TxReaction.RevertOnReturnFalse) {
96+
bool success = abi.decode(returnData, (bool));
97+
require(success, "tx returned boolean indicating internal failure");
98+
} else if (txReaction == TxReaction.CaptureBoolReturn) {
99+
execTransactionSuccess = abi.decode(returnData, (bool));
100+
} // else txReaction assumed to be IgnoreReturn, which does nothing else here
78101
}
79102

80103
// it is up to the caller to make sure that the msg.value of this method
81104
// equals the sum of all the values in the txs
82105
require(msg.value == sumTxsValues, "msg.value must equal sum of txs' values");
83106

84-
// final call in txs is assumed to be execTransaction
85-
execTransactionSuccess = abi.decode(lastReturnData, (bool));
86-
87107
emit CPKCreation(proxy, safeVersion, owner, salt);
88108
}
89109
}

contracts/solc-0.8/CPKFactoryFacade.sol

Lines changed: 76 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22
pragma solidity >=0.8.0;
33

44
import { Enum } from "./dep-ports/Enum.sol";
5-
import { CPKFactory, CPKFactoryTx } from "./CPKFactory.sol";
5+
import { CPKFactory, CPKFactoryTx, TxReaction } from "./CPKFactory.sol";
6+
import { SafeSignatureUtils } from "./SafeSignatureUtils.sol";
67

78
contract CPKFactoryFacade {
9+
using SafeSignatureUtils for bytes;
10+
811
CPKFactory immutable cpkFactory;
912
address immutable safeVersion;
1013
uint256 immutable salt;
@@ -38,22 +41,35 @@ contract CPKFactoryFacade {
3841
payable
3942
returns (bool)
4043
{
44+
// The signatures here aren't just the Gnosis Safe signatures,
45+
// but more data has been appended to them. In particular, the
46+
// format for the signatures is now like the following:
47+
// [inner sig][owner][outer sig]
48+
// where the inner signature is what is submitted to the
49+
// proxy as a first transaction to be executed after its creation,
50+
// the owner is the address of the owner of the new proxy,
51+
// left-padded to 32 bytes, and the outer signature is the overall
52+
// signature required by the CPKFactory.
53+
// The following process copies this signatures data into memory,
54+
// and then figures out the length of the inner signature,
55+
// and then extracts the owner from the signatures in memory,
56+
// and then overwrites the owner in memory with the length
57+
// of the outer signature, which is just the remaining
58+
// bytes of the signature. We end up with a situation in memory like
59+
// [inner sig]*[outer sig length][outer sig]
60+
// where the * is the pointer assigned to outerSig.
61+
bytes memory outerSig;
4162
address owner;
42-
// the following assembly block extracts the owner from the signature data
43-
// solium-disable-next-line security/no-inline-assembly
44-
assembly {
45-
// 0x124 is the start of the calldata word for the signature parameter
46-
// since it's a dynamic type, it stores the offset to the part of the calldata
47-
// that stores the actual data being sent as a signature so we load the
48-
// offset to the data, and then load the first word of the data which is
49-
// the length of the signature. Adding this offset to the signature data position
50-
// lets us grab the trailing word of the signature, which we will interpret
51-
// as the owner.
52-
let sigPos := calldataload(0x124)
53-
owner := and(
54-
calldataload(add(sigPos, calldataload(sigPos))),
55-
0xffffffffffffffffffffffffffffffffffffffff
56-
)
63+
uint innerSigLen;
64+
{
65+
bytes memory innerSig = signatures;
66+
innerSigLen = innerSig.actualLength();
67+
uint outerSigLen = signatures.length - innerSigLen - 0x20;
68+
assembly {
69+
outerSig := add(add(innerSig, 0x20), innerSigLen)
70+
owner := mload(outerSig)
71+
mstore(outerSig, outerSigLen)
72+
}
5773
}
5874

5975
CPKFactoryTx[] memory txs = new CPKFactoryTx[](2);
@@ -75,23 +91,58 @@ contract CPKFactoryFacade {
7591
")",
7692
owners, uint256(1), address(0), "",
7793
fallbackHandler, address(0), uint256(0), payable(0)
78-
)
94+
),
95+
reaction: TxReaction.IgnoreReturn
7996
});
8097
}
8198

82-
// msg.data works here as a substitute for encoding because this function's signature
83-
// exactly matches the execTransaction signature from the Gnosis Safe, so the calldata
84-
// encoding will be the same.
85-
txs[1] = CPKFactoryTx({
86-
value: msg.value,
87-
data: msg.data
88-
});
99+
{
100+
// trying to reencode the msg.data with the params except instead
101+
// of signatures using innerSig would be the obvious and readable approach
102+
// except we run into stack too deep errors that can't be circumvented yet,
103+
// so instead we copy the entire msg.data into memory and
104+
// mutate the signatures portion in it.
105+
bytes memory innerTxData = msg.data;
106+
assembly {
107+
// index param 9 is signatures, and 9 * 0x20 + 4 = 0x124
108+
// 0x124 from the start of the data portion of innerTxData
109+
// contains the offset to the start of the word
110+
// containing a further offset of where the signatures data
111+
// begins.
112+
// We add 0x20 to account for the word containing the length
113+
// of innerTxData, making a total offset of 0x144 to the offset data
114+
// Then we add that offset, a word for the overall length, and
115+
// the pointer to innerTxData, to arrive at a pointer to the signatures
116+
// inside innerTxData
117+
let sigs := add(innerTxData, add(mload(add(innerTxData, 0x144)), 0x20))
118+
// Since we know the length of the inner signature, we get the
119+
// size reduction we need by subtracting the inner signature length
120+
// from the size of all the signature data
121+
let sizeReduction := sub(mload(sigs), innerSigLen)
122+
mstore(sigs, innerSigLen)
123+
// The size reduction is then used to cut the outer signature out
124+
// of the msg.data.
125+
// This assumes that the signatures data is the last chunk of data
126+
// in the msg.data.
127+
// We can't keep the outer signature in the inner transaction data
128+
// because the outer signature signs the inner transaction data,
129+
// so keeping that there would make the outer signature impossible
130+
// to generate.
131+
mstore(innerTxData, sub(mload(innerTxData), sizeReduction))
132+
}
133+
txs[1] = CPKFactoryTx({
134+
value: msg.value,
135+
data: innerTxData,
136+
reaction: TxReaction.CaptureBoolReturn
137+
});
138+
}
89139

90140
return cpkFactory.createProxyAndExecTransaction{value: msg.value}(
91141
owner,
92142
safeVersion,
93143
salt,
94-
txs
144+
txs,
145+
outerSig
95146
);
96147
}
97148
}
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// SPDX-License-Identifier: GPL-3.0-only
2+
pragma solidity >=0.8.0;
3+
4+
import { ISignatureValidator, EIP1271_MAGIC_VALUE } from "./dep-ports/ISignatureValidator.sol";
5+
6+
library SafeSignatureUtils {
7+
// Adapted from SignatureDecoder
8+
function components(bytes memory signature)
9+
internal
10+
pure
11+
returns (uint8 v, bytes32 r, bytes32 s)
12+
{
13+
// The signature format is a compact form of:
14+
// {bytes32 r}{bytes32 s}{uint8 v}
15+
// Compact means, uint8 is not padded to 32 bytes.
16+
// solium-disable-next-line security/no-inline-assembly
17+
assembly {
18+
r := mload(add(signature, 0x20))
19+
s := mload(add(signature, 0x40))
20+
// Here we are loading the last 32 bytes, including 31 bytes
21+
// of 's'. There is no 'mload8' to do this.
22+
//
23+
// 'byte' is not working due to the Solidity parser, so lets
24+
// use the second best option, 'and'
25+
v := and(mload(add(signature, 0x41)), 0xff)
26+
}
27+
}
28+
29+
// Adapted from GnosisSafe
30+
function check(bytes memory signature, bytes32 dataHash, bytes memory data, address owner)
31+
internal
32+
view
33+
{
34+
// Check that the provided signature data is not too short
35+
require(signature.length >= 65, "Signature data too short");
36+
37+
uint8 v;
38+
bytes32 r;
39+
bytes32 s;
40+
(v, r, s) = components(signature);
41+
42+
address derivedOwner;
43+
// If v is 0 then it is a contract signature
44+
if (v == 0) {
45+
// When handling contract signatures the address of the contract is encoded into r
46+
derivedOwner = address(uint160(uint256(r)));
47+
48+
uint256 contractSignatureLen = requireContractSignatureLength(signature, uint256(s));
49+
require(uint256(s) + 32 + contractSignatureLen <= signature.length, "Invalid contract signature location: data not complete");
50+
51+
// Check signature
52+
bytes memory contractSignature;
53+
// solium-disable-next-line security/no-inline-assembly
54+
assembly {
55+
// The signature data for contract signatures is appended to the concatenated signatures and the offset is stored in s
56+
contractSignature := add(add(signature, s), 0x20)
57+
}
58+
require(ISignatureValidator(derivedOwner).isValidSignature(data, contractSignature) == EIP1271_MAGIC_VALUE, "Invalid contract signature provided");
59+
// If v is 1 then it is an approved hash
60+
} else if (v == 1) {
61+
// When handling approved hashes the address of the approver is encoded into r
62+
derivedOwner = address(uint160(uint256(r)));
63+
// Hashes are automatically approved by the sender of the message or when they have been pre-approved via a separate transaction
64+
require(msg.sender == derivedOwner);
65+
} else if (v > 30) {
66+
// To support eth_sign and similar we adjust v and hash the messageHash with the Ethereum message prefix before applying ecrecover
67+
derivedOwner = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
68+
} else {
69+
// Use ecrecover with the messageHash for EOA signatures
70+
derivedOwner = ecrecover(dataHash, v, r, s);
71+
}
72+
require (derivedOwner == owner, "Invalid owner provided");
73+
}
74+
75+
function requireContractSignatureLength(
76+
bytes memory signature,
77+
uint256 sigLoc
78+
)
79+
internal
80+
pure
81+
returns (uint256 contractSignatureLen)
82+
{
83+
// Check that signature data pointer (s) is not pointing inside the static part of the signatures bytes
84+
// This check is not completely accurate, since it is possible that more signatures than the threshold are send.
85+
// Here we only check that the pointer is not pointing inside the part that is being processed
86+
require(sigLoc >= 65, "Invalid contract signature location: inside static part");
87+
88+
// Check that signature data pointer (s) is in bounds (points to the length of data -> 32 bytes)
89+
require(sigLoc + 32 <= signature.length, "Invalid contract signature location: length not present");
90+
91+
// Check if the contract signature is in bounds: start of data is s + 32 and end is start + signature length
92+
// solium-disable-next-line security/no-inline-assembly
93+
assembly {
94+
contractSignatureLen := mload(add(add(signature, sigLoc), 0x20))
95+
}
96+
}
97+
98+
function actualLength(bytes memory signature)
99+
internal
100+
pure
101+
returns (uint256)
102+
{
103+
uint8 v;
104+
bytes32 r;
105+
bytes32 s;
106+
(v, r, s) = components(signature);
107+
108+
if (v == 0) {
109+
uint256 contractSignatureLen = requireContractSignatureLength(signature, uint256(s));
110+
return uint256(s) + 32 + contractSignatureLen;
111+
}
112+
113+
return 0x41;
114+
}
115+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// SPDX-License-Identifier: GPL-3.0-only
2+
pragma solidity >=0.8.0;
3+
4+
// bytes4(keccak256("isValidSignature(bytes,bytes)")
5+
bytes4 constant EIP1271_MAGIC_VALUE = 0x20c13b0b;
6+
7+
interface ISignatureValidator {
8+
9+
/**
10+
* @dev Should return whether the signature provided is valid for the provided data
11+
* @param _data Arbitrary length data signed on the behalf of address(this)
12+
* @param _signature Signature byte array associated with _data
13+
*
14+
* MUST return the bytes4 magic value 0x20c13b0b when function passes.
15+
* MUST NOT modify state (using STATICCALL for solc < 0.5, view modifier for solc > 0.5)
16+
* MUST allow external calls
17+
*/
18+
function isValidSignature(
19+
bytes memory _data,
20+
bytes memory _signature)
21+
external
22+
view
23+
returns (bytes4);
24+
}

0 commit comments

Comments
 (0)