Skip to content

Commit

Permalink
S ent from my iPhone
Browse files Browse the repository at this point in the history
  • Loading branch information
mmv08 committed Jul 13, 2023
2 parents 48ee396 + 5a6aa44 commit 0730ec9
Show file tree
Hide file tree
Showing 26 changed files with 342 additions and 215 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ jobs:
strategy:
fail-fast: false
matrix:
solidity: ["0.7.6", "0.8.2"]
solidity: ["0.7.6", "0.8.19"]
include:
- solidity: "0.8.2"
settings: '{"viaIR":true,"optimizer":{"enabled":true,"runs":10000}}'
- solidity: "0.8.19"
settings: '{"viaIR":true,"optimizer":{"enabled":true,"runs":1000000}}'
env:
SOLIDITY_VERSION: ${{ matrix.solidity }}
SOLIDITY_SETTINGS: ${{ matrix.settings }}
Expand All @@ -78,4 +78,4 @@ jobs:
with:
path: "**/node_modules"
key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
- run: (yarn --frozen-lockfile && yarn build && yarn hardhat codesize --contractname Safe && yarn benchmark) || echo "Benchmark failed"
- run: (yarn --frozen-lockfile && yarn build && yarn hardhat codesize --contractname Safe && yarn benchmark) || echo "Benchmark failed"
36 changes: 18 additions & 18 deletions certora/applyHarness.patch
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
diff -druN Safe.sol Safe.sol
--- Safe.sol 2023-05-16 15:08:39
+++ Safe.sol 2023-05-25 16:23:56
@@ -76,7 +76,7 @@
* so we create a Safe with 0 owners and threshold 1.
* This is an unusable Safe, perfect for the singleton
*/
- threshold = 1;
+ // threshold = 1; MUNGED: remove and add to constructor of the harness
}

/**
diff -druN base/Executor.sol base/Executor.sol
--- base/Executor.sol 2023-05-16 15:08:39
+++ base/Executor.sol 2023-05-25 16:23:31
@@ -25,11 +25,9 @@
Enum.Operation operation,
--- base/Executor.sol 2023-06-30 15:32:21.392860349 +0200
+++ base/Executor.sol 2023-06-30 15:37:58.671801994 +0200
@@ -26,11 +26,8 @@
uint256 txGas
) internal returns (bool success) {
+ // MUNGED lets just be a bit more optimistic, `execute` does nothing for `DELEGATECALL` and always returns true
if (operation == Enum.Operation.DelegateCall) {
- // solhint-disable-next-line no-inline-assembly
- /// @solidity memory-safe-assembly
- assembly {
- success := delegatecall(txGas, to, add(data, 0x20), mload(data), 0, 0)
- }
+ // MUNGED lets just be a bit more optimistic, `execute` does nothing for `DELEGATECALL` and always returns true
+ return true;
} else {
// solhint-disable-next-line no-inline-assembly
assembly {
/// @solidity memory-safe-assembly
diff -druN Safe.sol Safe.sol
--- Safe.sol 2023-06-30 15:32:21.392860349 +0200
+++ Safe.sol 2023-06-30 15:37:17.198953773 +0200
@@ -76,7 +76,7 @@
* so we create a Safe with 0 owners and threshold 1.
* This is an unusable Safe, perfect for the singleton
*/
- threshold = 1;
+ // threshold = 1; MUNGED: remove and add to constructor of the harness
}

/**
1 change: 0 additions & 1 deletion certora/specs/Safe.spec
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
methods {
//
function getThreshold() external returns (uint256) envfree;
function disableModule(address,address) external;
function nonce() external returns (uint256) envfree;
Expand Down
64 changes: 31 additions & 33 deletions contracts/Safe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,6 @@ contract Safe is
bytes32 txHash;
// Use scope here to limit variable lifetime and prevent `stack too deep` errors
{
// Increase nonce and execute transaction.
nonce++;

txHash = getTransactionHash( // Transaction info
to,
value,
Expand All @@ -166,7 +163,7 @@ contract Safe is
gasToken,
refundReceiver,
// Signature info
nonce
nonce++
);
checkSignatures(txHash, "", signatures);
}
Expand All @@ -191,6 +188,7 @@ contract Safe is
);
}
}

// We require some gas to emit the events (at least 2500) after the execution and some to perform code until the execution (500)
// We also include the 1/64 in the check that is not send along with a call to counteract potential shortings because of EIP-150
require(gasleft() >= ((safeTxGas * 64) / 63).max(safeTxGas + 2500) + 500, "GS010");
Expand Down Expand Up @@ -237,9 +235,10 @@ contract Safe is
// solhint-disable-next-line avoid-tx-origin
address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
if (gasToken == address(0)) {
// For ETH we will only adjust the gas price to not be higher than the actual used gas price
// For native tokens, we will only adjust the gas price to not be higher than the actually used gas price
payment = gasUsed.add(baseGas).mul(gasPrice < tx.gasprice ? gasPrice : tx.gasprice);
require(receiver.send(payment), "GS011");
(bool refundSuccess, ) = receiver.call{value: payment}("");
require(refundSuccess, "GS011");
} else {
payment = gasUsed.add(baseGas).mul(gasPrice);
require(transferToken(gasToken, receiver, payment), "GS012");
Expand All @@ -265,6 +264,22 @@ contract Safe is
* @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
* @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
* @param dataHash Hash of the data (could be either a message hash or transaction hash)
* @param data That should be signed (this is passed to an external validator contract)
* @param signatures Signature data that should be verified.
* Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
* @param requiredSignatures Amount of required valid signatures.
*/
function checkNSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures, uint256 requiredSignatures) public view {
return checkNSignatures(msg.sender, dataHash, data, signatures, requiredSignatures);
}

/**
* @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
* @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
* The data parameter (bytes) is not used since v1.5.0 as it is not required anymore. Prior to v1.5.0,
* data parameter was used in contract signature validation flow using legacy EIP-1271.
* Version v1.5.0, uses dataHash parameter instead of data with updated EIP-1271 implementation.
* @param dataHash Hash of the data (could be either a message hash or transaction hash)
* @param signatures Signature data that should be verified.
* Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
* @param requiredSignatures Amount of required valid signatures.
Expand Down Expand Up @@ -303,6 +318,7 @@ contract Safe is
// Check if the contract signature is in bounds: start of data is s + 32 and end is start + signature length
uint256 contractSignatureLen;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
contractSignatureLen := mload(add(add(signatures, s), 0x20))
}
Expand All @@ -311,6 +327,7 @@ contract Safe is
// Check signature
bytes memory contractSignature;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
// The signature data for contract signatures is appended to the concatenated signatures and the offset is stored in s
contractSignature := add(add(signatures, s), 0x20)
Expand All @@ -336,19 +353,6 @@ contract Safe is
}
}

/**
* @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
* @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
* @param dataHash Hash of the data (could be either a message hash or transaction hash)
* @param data That should be signed (this is passed to an external validator contract)
* @param signatures Signature data that should be verified.
* Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
* @param requiredSignatures Amount of required valid signatures.
*/
function checkNSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures, uint256 requiredSignatures) public view {
return checkNSignatures(msg.sender, dataHash, data, signatures, requiredSignatures);
}

/**
* @notice Marks hash `hashToApprove` as approved.
* @dev This can be used with a pre-approved hash transaction signature.
Expand All @@ -362,24 +366,18 @@ contract Safe is
}

/**
* @notice Returns the ID of the chain the contract is currently deployed on.
* @return The ID of the current chain as a uint256.
* @dev Returns the domain separator for this contract, as defined in the EIP-712 standard.
* @return bytes32 The domain separator hash.
*/
function getChainId() public view returns (uint256) {
uint256 id;
function domainSeparator() public view returns (bytes32) {
uint256 chainId;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
id := chainid()
chainId := chainid()
}
return id;
}

/**
* @dev Returns the domain separator for this contract, as defined in the EIP-712 standard.
* @return bytes32 The domain separator hash.
*/
function domainSeparator() public view returns (bytes32) {
return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, getChainId(), this));
return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, chainId, this));
}

/**
Expand Down Expand Up @@ -407,7 +405,7 @@ contract Safe is
address gasToken,
address refundReceiver,
uint256 _nonce
) public view returns (bytes memory) {
) private view returns (bytes memory) {
bytes32 safeTxHash = keccak256(
abi.encode(
SAFE_TX_TYPEHASH,
Expand Down
1 change: 1 addition & 0 deletions contracts/accessors/SimulateTxAccessor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ contract SimulateTxAccessor is Executor {
success = execute(to, value, data, operation, gasleft());
estimate = startGas - gasleft();
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 53 in contracts/accessors/SimulateTxAccessor.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 53 in contracts/accessors/SimulateTxAccessor.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
// Load free memory location
let ptr := mload(0x40)
Expand Down
2 changes: 2 additions & 0 deletions contracts/base/Executor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ abstract contract Executor {
) internal returns (bool success) {
if (operation == Enum.Operation.DelegateCall) {
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 31 in contracts/base/Executor.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 31 in contracts/base/Executor.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
success := delegatecall(txGas, to, add(data, 0x20), mload(data), 0, 0)
}
} else {
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 37 in contracts/base/Executor.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 37 in contracts/base/Executor.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
success := call(txGas, to, value, add(data, 0x20), mload(data), 0, 0)
}
Expand Down
30 changes: 24 additions & 6 deletions contracts/base/FallbackManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ abstract contract FallbackManager is SelfAuthorized {

bytes32 slot = FALLBACK_HANDLER_STORAGE_SLOT;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 39 in contracts/base/FallbackManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 39 in contracts/base/FallbackManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
sstore(slot, handler)
}
Expand All @@ -61,22 +62,39 @@ abstract contract FallbackManager is SelfAuthorized {
fallback() external {
bytes32 slot = FALLBACK_HANDLER_STORAGE_SLOT;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 66 in contracts/base/FallbackManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 66 in contracts/base/FallbackManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
// When compiled with the optimizer, the compiler relies on a certain assumptions on how the
// memory is used, therefore we need to guarantee memory safety (keeping the free memory point 0x40 slot intact,
// not going beyond the scratch space, etc)
// Solidity docs: https://docs.soliditylang.org/en/latest/assembly.html#memory-safety
function allocate(length) -> pos {
pos := mload(0x40)
mstore(0x40, add(pos, length))
}

let handler := sload(slot)
if iszero(handler) {
return(0, 0)
}
calldatacopy(0, 0, calldatasize())

let calldataPtr := allocate(calldatasize())
calldatacopy(calldataPtr, 0, calldatasize())

// The msg.sender address is shifted to the left by 12 bytes to remove the padding
// Then the address without padding is stored right after the calldata
mstore(calldatasize(), shl(96, caller()))
let senderPtr := allocate(20)
mstore(senderPtr, shl(96, caller()))

// Add 20 bytes for the address appended add the end
let success := call(gas(), handler, 0, 0, add(calldatasize(), 20), 0, 0)
returndatacopy(0, 0, returndatasize())
let success := call(gas(), handler, 0, calldataPtr, add(calldatasize(), 20), 0, 0)

let returnDataPtr := allocate(returndatasize())
returndatacopy(returnDataPtr, 0, returndatasize())
if iszero(success) {
revert(0, returndatasize())
revert(returnDataPtr, returndatasize())
}
return(0, returndatasize())
return(returnDataPtr, returndatasize())
}
}
}
2 changes: 2 additions & 0 deletions contracts/base/GuardManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ abstract contract GuardManager is SelfAuthorized {
}
bytes32 slot = GUARD_STORAGE_SLOT;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 60 in contracts/base/GuardManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 60 in contracts/base/GuardManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
sstore(slot, guard)
}
Expand All @@ -72,6 +73,7 @@ abstract contract GuardManager is SelfAuthorized {
function getGuard() internal view returns (address guard) {
bytes32 slot = GUARD_STORAGE_SLOT;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 77 in contracts/base/GuardManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 77 in contracts/base/GuardManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
guard := sload(slot)
}
Expand Down
3 changes: 3 additions & 0 deletions contracts/base/ModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ abstract contract ModuleManager is SelfAuthorized, Executor {
) public returns (bool success, bytes memory returnData) {
success = execTransactionFromModule(to, value, data, operation);
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 113 in contracts/base/ModuleManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 113 in contracts/base/ModuleManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
// Load free memory location
let ptr := mload(0x40)
Expand Down Expand Up @@ -169,6 +170,7 @@ abstract contract ModuleManager is SelfAuthorized, Executor {
}
// Set correct size of returned array
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 174 in contracts/base/ModuleManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 174 in contracts/base/ModuleManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
mstore(array, moduleCount)
}
Expand All @@ -183,6 +185,7 @@ abstract contract ModuleManager is SelfAuthorized, Executor {
function isContract(address account) internal view returns (bool) {
uint256 size;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {

Check warning on line 189 in contracts/base/ModuleManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 189 in contracts/base/ModuleManager.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
size := extcodesize(account)
}
Expand Down
1 change: 1 addition & 0 deletions contracts/common/SecuredTokenTransfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ abstract contract SecuredTokenTransfer {
// 0xa9059cbb - keccack("transfer(address,uint256)")
bytes memory data = abi.encodeWithSelector(0xa9059cbb, receiver, amount);
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
// We write the return value to scratch space.
// See https://docs.soliditylang.org/en/v0.7.6/internals/layout_in_memory.html#layout-in-memory
Expand Down
1 change: 1 addition & 0 deletions contracts/common/SignatureDecoder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ abstract contract SignatureDecoder {
*/
function signatureSplit(bytes memory signatures, uint256 pos) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
let signaturePos := mul(0x41, pos)
r := mload(add(signatures, add(signaturePos, 0x20)))
Expand Down
13 changes: 8 additions & 5 deletions contracts/common/StorageAccessible.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ abstract contract StorageAccessible {
bytes memory result = new bytes(length * 32);
for (uint256 index = 0; index < length; index++) {
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
let word := sload(add(offset, index))
mstore(add(add(result, 0x20), mul(index, 0x20)), word)
Expand All @@ -39,13 +40,15 @@ abstract contract StorageAccessible {
*/
function simulateAndRevert(address targetContract, bytes memory calldataPayload) external {
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
let success := delegatecall(gas(), targetContract, add(calldataPayload, 0x20), mload(calldataPayload), 0, 0)

mstore(0x00, success)
mstore(0x20, returndatasize())
returndatacopy(0x40, 0, returndatasize())
revert(0, add(returndatasize(), 0x40))
// Load free memory location
let ptr := mload(0x40)
mstore(ptr, success)
mstore(add(ptr, 0x20), returndatasize())
returndatacopy(add(ptr, 0x40), 0, returndatasize())
revert(ptr, add(returndatasize(), 0x40))
}
}
}
1 change: 1 addition & 0 deletions contracts/examples/guards/ReentrancyTransactionGuard.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ contract ReentrancyTransactionGuard is BaseGuard {
function getGuard() internal pure returns (GuardValue storage guard) {
bytes32 slot = GUARD_STORAGE_SLOT;
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
guard.slot := slot
}
Expand Down
1 change: 1 addition & 0 deletions contracts/handler/CompatibilityFallbackHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
calldataPayload;

// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
let internalCalldata := mload(0x40)
/**
Expand Down
1 change: 1 addition & 0 deletions contracts/handler/HandlerContext.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ abstract contract HandlerContext {
function _msgSender() internal pure returns (address sender) {
// The assembly code is more direct than the Solidity version using `abi.decode`.
// solhint-disable-next-line no-inline-assembly
/// @solidity memory-safe-assembly
assembly {
sender := shr(96, calldataload(sub(calldatasize(), 20)))
}
Expand Down
Loading

0 comments on commit 0730ec9

Please sign in to comment.