diff --git a/contracts/Safe.sol b/contracts/Safe.sol index 82afe3be5..b08611354 100644 --- a/contracts/Safe.sol +++ b/contracts/Safe.sol @@ -179,9 +179,9 @@ contract Safe is /* solhint-disable no-inline-assembly */ /// @solidity memory-safe-assembly assembly { - let p := mload(0x40) - returndatacopy(p, 0, returndatasize()) - revert(p, returndatasize()) + let ptr := mload(0x40) + returndatacopy(ptr, 0, returndatasize()) + revert(ptr, returndatasize()) } /* solhint-enable no-inline-assembly */ } @@ -403,9 +403,10 @@ contract Safe is ) public view override returns (bytes32 txHash) { bytes32 domainHash = domainSeparator(); - // We opted out for using assembly code here, because the way Solidity compiler we use (0.7.6) - // allocates memory is inefficient. We only need to allocate memory for temporary variables to be used in the keccak256 call. + // We opted for using assembly code here, because the way Solidity compiler we use (0.7.6) allocates memory is + // inefficient. We do not need to allocate memory for temporary variables to be used in the keccak256 call. /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { // Get the free memory pointer let ptr := mload(0x40) diff --git a/contracts/base/ModuleManager.sol b/contracts/base/ModuleManager.sol index 9f25eafad..b3985be85 100644 --- a/contracts/base/ModuleManager.sol +++ b/contracts/base/ModuleManager.sol @@ -260,10 +260,12 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager { revertWithError("GS301"); bytes32 slot = MODULE_GUARD_STORAGE_SLOT; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { sstore(slot, moduleGuard) } + /* solhint-enable no-inline-assembly */ emit ChangedModuleGuard(moduleGuard); } @@ -273,10 +275,12 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager { */ function getModuleGuard() internal view returns (address moduleGuard) { bytes32 slot = MODULE_GUARD_STORAGE_SLOT; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { moduleGuard := sload(slot) } + /* solhint-enable no-inline-assembly */ } /** diff --git a/contracts/handler/extensible/MarshalLib.sol b/contracts/handler/extensible/MarshalLib.sol index ca33cf678..b55436e7a 100644 --- a/contracts/handler/extensible/MarshalLib.sol +++ b/contracts/handler/extensible/MarshalLib.sol @@ -33,12 +33,14 @@ library MarshalLib { * @return handler The address of the handler contract implementing the `IFallbackMethod` or `IStaticFallbackMethod` interface */ function decode(bytes32 data) internal pure returns (bool isStatic, address handler) { - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { // set isStatic to true if the left-most byte of the data is 0x00 isStatic := iszero(shr(248, data)) handler := shr(96, shl(96, data)) } + /* solhint-enable no-inline-assembly */ } /** @@ -49,12 +51,14 @@ library MarshalLib { * @return handler The address of the handler contract implementing the `IFallbackMethod` or `IStaticFallbackMethod` interface */ function decodeWithSelector(bytes32 data) internal pure returns (bool isStatic, bytes4 selector, address handler) { - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { // set isStatic to true if the left-most byte of the data is 0x00 isStatic := iszero(shr(248, data)) handler := shr(96, shl(96, data)) selector := shl(168, shr(160, data)) } + /* solhint-enable no-inline-assembly */ } } diff --git a/contracts/handler/extensible/SignatureVerifierMuxer.sol b/contracts/handler/extensible/SignatureVerifierMuxer.sol index 17b3fc43e..8bc772d03 100644 --- a/contracts/handler/extensible/SignatureVerifierMuxer.sol +++ b/contracts/handler/extensible/SignatureVerifierMuxer.sol @@ -107,10 +107,12 @@ abstract contract SignatureVerifierMuxer is ExtensibleBase, ERC1271, ISignatureV // Check if the signature is for an `ISafeSignatureVerifier` and if it is valid for the domain. if (signature.length >= 4) { bytes4 sigSelector; - // solhint-disable-next-line no-inline-assembly + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { sigSelector := shl(224, shr(224, calldataload(signature.offset))) } + /* solhint-enable no-inline-assembly */ // Guard against short signatures that would cause abi.decode to revert. if (sigSelector == SAFE_SIGNATURE_MAGIC_VALUE && signature.length >= 68) { diff --git a/contracts/libraries/MultiSend.sol b/contracts/libraries/MultiSend.sol index 82b849e38..399148a4e 100644 --- a/contracts/libraries/MultiSend.sol +++ b/contracts/libraries/MultiSend.sol @@ -30,6 +30,7 @@ contract MultiSend { function multiSend(bytes memory transactions) public payable { require(address(this) != MULTISEND_SINGLETON, "MultiSend should only be called via delegatecall"); /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { let length := mload(transactions) let i := 0x20 @@ -61,9 +62,10 @@ contract MultiSend { case 1 { success := delegatecall(gas(), to, data, dataLength, 0, 0) } - if eq(success, 0) { - returndatacopy(0, 0, returndatasize()) - revert(0, returndatasize()) + if iszero(success) { + let ptr := mload(0x40) + returndatacopy(ptr, 0, returndatasize()) + revert(ptr, returndatasize()) } // Next entry starts at 85 byte + data length i := add(i, add(0x55, dataLength)) diff --git a/contracts/libraries/MultiSendCallOnly.sol b/contracts/libraries/MultiSendCallOnly.sol index fc4dd2f2e..2c30e8821 100644 --- a/contracts/libraries/MultiSendCallOnly.sol +++ b/contracts/libraries/MultiSendCallOnly.sol @@ -24,6 +24,7 @@ contract MultiSendCallOnly { */ function multiSend(bytes memory transactions) public payable { /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { let length := mload(transactions) let i := 0x20 @@ -56,9 +57,10 @@ contract MultiSendCallOnly { case 1 { revert(0, 0) } - if eq(success, 0) { - returndatacopy(0, 0, returndatasize()) - revert(0, returndatasize()) + if iszero(success) { + let ptr := mload(0x40) + returndatacopy(ptr, 0, returndatasize()) + revert(ptr, returndatasize()) } // Next entry starts at 85 byte + data length i := add(i, add(0x55, dataLength)) diff --git a/contracts/proxies/SafeProxy.sol b/contracts/proxies/SafeProxy.sol index 73f7daee5..a6b3cbbe5 100644 --- a/contracts/proxies/SafeProxy.sol +++ b/contracts/proxies/SafeProxy.sol @@ -31,7 +31,11 @@ contract SafeProxy { /// @dev Fallback function forwards all transactions and returns all received return data. fallback() external payable { - // solhint-disable-next-line no-inline-assembly + // Note that this assembly block is **intentionally** not marked as memory-safe. First of all, it isn't memory + // safe to begin with, and turning this into memory-safe assembly would just make it less gas efficient. + // Additionally, we noticed that converting this to memory-safe assembly had no affect on optimizations of other + // contracts (as it always gets compiled alone in its own compilation unit anyway). + /* solhint-disable no-inline-assembly */ assembly { let _singleton := sload(0) // 0xa619486e == keccak("masterCopy()"). The value is right padded to 32-bytes with 0s @@ -42,10 +46,11 @@ contract SafeProxy { calldatacopy(0, 0, calldatasize()) let success := delegatecall(gas(), _singleton, 0, calldatasize(), 0, 0) returndatacopy(0, 0, returndatasize()) - if eq(success, 0) { + if iszero(success) { revert(0, returndatasize()) } return(0, returndatasize()) } + /* solhint-enable no-inline-assembly */ } } diff --git a/contracts/test/DelegateCaller.sol b/contracts/test/DelegateCaller.sol index 6a513f5ca..969d0623f 100644 --- a/contracts/test/DelegateCaller.sol +++ b/contracts/test/DelegateCaller.sol @@ -14,9 +14,11 @@ contract DelegateCaller { (success, returnData) = _called.delegatecall(_calldata); if (!success) { /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly assembly { - returndatacopy(0, 0, returndatasize()) - revert(0, returndatasize()) + let ptr := mload(0x40) + returndatacopy(ptr, 0, returndatasize()) + revert(ptr, returndatasize()) } /* solhint-enable no-inline-assembly */ } diff --git a/docs/safe_tx_gas.md b/docs/safe_tx_gas.md index cc8ac592f..a27ad5fa0 100644 --- a/docs/safe_tx_gas.md +++ b/docs/safe_tx_gas.md @@ -56,9 +56,9 @@ if (!success && safeTxGas == 0 && gasPrice == 0) { /* solhint-disable no-inline-assembly */ /// @solidity memory-safe-assembly assembly { - let p := mload(0x40) - returndatacopy(p, 0, returndatasize()) - revert(p, returndatasize()) + let ptr := mload(0x40) + returndatacopy(ptr, 0, returndatasize()) + revert(ptr, returndatasize()) } /* solhint-enable no-inline-assembly */ }