Skip to content

Commit

Permalink
Found an approach that might work
Browse files Browse the repository at this point in the history
  • Loading branch information
cag committed Dec 30, 2020
1 parent b2e18c0 commit 4368c56
Show file tree
Hide file tree
Showing 4 changed files with 246 additions and 36 deletions.
42 changes: 31 additions & 11 deletions contracts/solc-0.8/CPKFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,23 @@ pragma solidity >=0.8.0;

import { IGnosisSafeProxyFactory } from "./dep-ports/IGnosisSafeProxyFactory.sol";
import { ProxyImplSetter } from "./ProxyImplSetter.sol";
import { SafeSignatureUtils } from "./SafeSignatureUtils.sol";

enum TxReaction {
RevertOnReturnFalse,
CaptureBoolReturn,
IgnoreReturn
}

struct CPKFactoryTx {
uint value;
bytes data;
TxReaction reaction;
}

contract CPKFactory {
using SafeSignatureUtils for bytes;

event CPKCreation(
address indexed proxy,
address initialImpl,
Expand Down Expand Up @@ -40,12 +50,17 @@ contract CPKFactory {
address owner,
address safeVersion,
uint256 salt,
CPKFactoryTx[] calldata txs
CPKFactoryTx[] calldata txs,
bytes calldata signature
)
external
payable
returns (bool execTransactionSuccess)
{
bytes memory data = abi.encode(safeVersion, salt, txs);
bytes32 dataHash = keccak256(data);
signature.check(dataHash, data, owner);

bytes32 saltNonce = keccak256(abi.encode(owner, salt));

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

uint sumTxsValues = 0;
bytes memory lastReturnData;
for (uint i = 0; i < txs.length; i++) {
bool txSuccess;
bytes memory returnData;
uint txValue = txs[i].value;
sumTxsValues += txValue;
(txSuccess, lastReturnData) = proxy.call{value: txValue}(txs[i].data);
(txSuccess, returnData) = proxy.call{value: txValue}(txs[i].data);
assembly {
// txSuccess == 0 means the call failed
if iszero(txSuccess) {
// The revert data begins one word after the lastReturnData pointer.
// At the location lastReturnData in memory, the length of the bytes is stored.
// This differs from the high-level revert(string(lastReturnData))
// as the high-level version encodes the lastReturnData in a Error(string) object.
// The revert data begins one word after the returnData pointer.
// At the location returnData in memory, the length of the bytes is stored.
// This differs from the high-level revert(string(returnData))
// as the high-level version encodes the returnData in a Error(string) object.
// We want to avoid that because the underlying call should have already
// formatted the data in an Error(string) object
revert(add(0x20, lastReturnData), mload(lastReturnData))
revert(add(0x20, returnData), mload(returnData))
}
}

TxReaction txReaction = txs[i].reaction;
if (txReaction == TxReaction.RevertOnReturnFalse) {
bool success = abi.decode(returnData, (bool));
require(success, "tx returned boolean indicating internal failure");
} else if (txReaction == TxReaction.CaptureBoolReturn) {
execTransactionSuccess = abi.decode(returnData, (bool));
} // else txReaction assumed to be IgnoreReturn, which does nothing else here
}

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

// final call in txs is assumed to be execTransaction
execTransactionSuccess = abi.decode(lastReturnData, (bool));

emit CPKCreation(proxy, safeVersion, owner, salt);
}
}
101 changes: 76 additions & 25 deletions contracts/solc-0.8/CPKFactoryFacade.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
pragma solidity >=0.8.0;

import { Enum } from "./dep-ports/Enum.sol";
import { CPKFactory, CPKFactoryTx } from "./CPKFactory.sol";
import { CPKFactory, CPKFactoryTx, TxReaction } from "./CPKFactory.sol";
import { SafeSignatureUtils } from "./SafeSignatureUtils.sol";

contract CPKFactoryFacade {
using SafeSignatureUtils for bytes;

CPKFactory immutable cpkFactory;
address immutable safeVersion;
uint256 immutable salt;
Expand Down Expand Up @@ -38,22 +41,35 @@ contract CPKFactoryFacade {
payable
returns (bool)
{
// The signatures here aren't just the Gnosis Safe signatures,
// but more data has been appended to them. In particular, the
// format for the signatures is now like the following:
// [inner sig][owner][outer sig]
// where the inner signature is what is submitted to the
// proxy as a first transaction to be executed after its creation,
// the owner is the address of the owner of the new proxy,
// left-padded to 32 bytes, and the outer signature is the overall
// signature required by the CPKFactory.
// The following process copies this signatures data into memory,
// and then figures out the length of the inner signature,
// and then extracts the owner from the signatures in memory,
// and then overwrites the owner in memory with the length
// of the outer signature, which is just the remaining
// bytes of the signature. We end up with a situation in memory like
// [inner sig]*[outer sig length][outer sig]
// where the * is the pointer assigned to outerSig.
bytes memory outerSig;
address owner;
// the following assembly block extracts the owner from the signature data
// solium-disable-next-line security/no-inline-assembly
assembly {
// 0x124 is the start of the calldata word for the signature parameter
// since it's a dynamic type, it stores the offset to the part of the calldata
// that stores the actual data being sent as a signature so we load the
// offset to the data, and then load the first word of the data which is
// the length of the signature. Adding this offset to the signature data position
// lets us grab the trailing word of the signature, which we will interpret
// as the owner.
let sigPos := calldataload(0x124)
owner := and(
calldataload(add(sigPos, calldataload(sigPos))),
0xffffffffffffffffffffffffffffffffffffffff
)
uint innerSigLen;
{
bytes memory innerSig = signatures;
innerSigLen = innerSig.actualLength();
uint outerSigLen = signatures.length - innerSigLen - 0x20;
assembly {
outerSig := add(add(innerSig, 0x20), innerSigLen)
owner := mload(outerSig)
mstore(outerSig, outerSigLen)
}
}

CPKFactoryTx[] memory txs = new CPKFactoryTx[](2);
Expand All @@ -75,23 +91,58 @@ contract CPKFactoryFacade {
")",
owners, uint256(1), address(0), "",
fallbackHandler, address(0), uint256(0), payable(0)
)
),
reaction: TxReaction.IgnoreReturn
});
}

// msg.data works here as a substitute for encoding because this function's signature
// exactly matches the execTransaction signature from the Gnosis Safe, so the calldata
// encoding will be the same.
txs[1] = CPKFactoryTx({
value: msg.value,
data: msg.data
});
{
// trying to reencode the msg.data with the params except instead
// of signatures using innerSig would be the obvious and readable approach
// except we run into stack too deep errors that can't be circumvented yet,
// so instead we copy the entire msg.data into memory and
// mutate the signatures portion in it.
bytes memory innerTxData = msg.data;
assembly {
// index param 9 is signatures, and 9 * 0x20 + 4 = 0x124
// 0x124 from the start of the data portion of innerTxData
// contains the offset to the start of the word
// containing a further offset of where the signatures data
// begins.
// We add 0x20 to account for the word containing the length
// of innerTxData, making a total offset of 0x144 to the offset data
// Then we add that offset, a word for the overall length, and
// the pointer to innerTxData, to arrive at a pointer to the signatures
// inside innerTxData
let sigs := add(innerTxData, add(mload(add(innerTxData, 0x144)), 0x20))
// Since we know the length of the inner signature, we get the
// size reduction we need by subtracting the inner signature length
// from the size of all the signature data
let sizeReduction := sub(mload(sigs), innerSigLen)
mstore(sigs, innerSigLen)
// The size reduction is then used to cut the outer signature out
// of the msg.data.
// This assumes that the signatures data is the last chunk of data
// in the msg.data.
// We can't keep the outer signature in the inner transaction data
// because the outer signature signs the inner transaction data,
// so keeping that there would make the outer signature impossible
// to generate.
mstore(innerTxData, sub(mload(innerTxData), sizeReduction))
}
txs[1] = CPKFactoryTx({
value: msg.value,
data: innerTxData,
reaction: TxReaction.CaptureBoolReturn
});
}

return cpkFactory.createProxyAndExecTransaction{value: msg.value}(
owner,
safeVersion,
salt,
txs
txs,
outerSig
);
}
}
115 changes: 115 additions & 0 deletions contracts/solc-0.8/SafeSignatureUtils.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity >=0.8.0;

import { ISignatureValidator, EIP1271_MAGIC_VALUE } from "./dep-ports/ISignatureValidator.sol";

library SafeSignatureUtils {
// Adapted from SignatureDecoder
function components(bytes memory signature)
internal
pure
returns (uint8 v, bytes32 r, bytes32 s)
{
// The signature format is a compact form of:
// {bytes32 r}{bytes32 s}{uint8 v}
// Compact means, uint8 is not padded to 32 bytes.
// solium-disable-next-line security/no-inline-assembly
assembly {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
// Here we are loading the last 32 bytes, including 31 bytes
// of 's'. There is no 'mload8' to do this.
//
// 'byte' is not working due to the Solidity parser, so lets
// use the second best option, 'and'
v := and(mload(add(signature, 0x41)), 0xff)
}
}

// Adapted from GnosisSafe
function check(bytes memory signature, bytes32 dataHash, bytes memory data, address owner)
internal
view
{
// Check that the provided signature data is not too short
require(signature.length >= 65, "Signature data too short");

uint8 v;
bytes32 r;
bytes32 s;
(v, r, s) = components(signature);

address derivedOwner;
// If v is 0 then it is a contract signature
if (v == 0) {
// When handling contract signatures the address of the contract is encoded into r
derivedOwner = address(uint160(uint256(r)));

uint256 contractSignatureLen = requireContractSignatureLength(signature, uint256(s));
require(uint256(s) + 32 + contractSignatureLen <= signature.length, "Invalid contract signature location: data not complete");

// Check signature
bytes memory contractSignature;
// solium-disable-next-line security/no-inline-assembly
assembly {
// The signature data for contract signatures is appended to the concatenated signatures and the offset is stored in s
contractSignature := add(add(signature, s), 0x20)
}
require(ISignatureValidator(derivedOwner).isValidSignature(data, contractSignature) == EIP1271_MAGIC_VALUE, "Invalid contract signature provided");
// If v is 1 then it is an approved hash
} else if (v == 1) {
// When handling approved hashes the address of the approver is encoded into r
derivedOwner = address(uint160(uint256(r)));
// Hashes are automatically approved by the sender of the message or when they have been pre-approved via a separate transaction
require(msg.sender == derivedOwner);
} else if (v > 30) {
// To support eth_sign and similar we adjust v and hash the messageHash with the Ethereum message prefix before applying ecrecover
derivedOwner = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
} else {
// Use ecrecover with the messageHash for EOA signatures
derivedOwner = ecrecover(dataHash, v, r, s);
}
require (derivedOwner == owner, "Invalid owner provided");
}

function requireContractSignatureLength(
bytes memory signature,
uint256 sigLoc
)
internal
pure
returns (uint256 contractSignatureLen)
{
// Check that signature data pointer (s) is not pointing inside the static part of the signatures bytes
// This check is not completely accurate, since it is possible that more signatures than the threshold are send.
// Here we only check that the pointer is not pointing inside the part that is being processed
require(sigLoc >= 65, "Invalid contract signature location: inside static part");

// Check that signature data pointer (s) is in bounds (points to the length of data -> 32 bytes)
require(sigLoc + 32 <= signature.length, "Invalid contract signature location: length not present");

// Check if the contract signature is in bounds: start of data is s + 32 and end is start + signature length
// solium-disable-next-line security/no-inline-assembly
assembly {
contractSignatureLen := mload(add(add(signature, sigLoc), 0x20))
}
}

function actualLength(bytes memory signature)
internal
pure
returns (uint256)
{
uint8 v;
bytes32 r;
bytes32 s;
(v, r, s) = components(signature);

if (v == 0) {
uint256 contractSignatureLen = requireContractSignatureLength(signature, uint256(s));
return uint256(s) + 32 + contractSignatureLen;
}

return 0x41;
}
}
24 changes: 24 additions & 0 deletions contracts/solc-0.8/dep-ports/ISignatureValidator.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity >=0.8.0;

// bytes4(keccak256("isValidSignature(bytes,bytes)")
bytes4 constant EIP1271_MAGIC_VALUE = 0x20c13b0b;

interface ISignatureValidator {

/**
* @dev Should return whether the signature provided is valid for the provided data
* @param _data Arbitrary length data signed on the behalf of address(this)
* @param _signature Signature byte array associated with _data
*
* MUST return the bytes4 magic value 0x20c13b0b when function passes.
* MUST NOT modify state (using STATICCALL for solc < 0.5, view modifier for solc > 0.5)
* MUST allow external calls
*/
function isValidSignature(
bytes memory _data,
bytes memory _signature)
external
view
returns (bytes4);
}

0 comments on commit 4368c56

Please sign in to comment.