Review based safe-memory changes

safe-global · Dec 22, 2023 · 9166949 · 9166949
1 parent 534e3f4
commit 9166949
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 29 deletions.
diff --git a/contracts/base/FallbackManager.sol b/contracts/base/FallbackManager.sol
@@ -79,23 +79,23 @@ abstract contract FallbackManager is SelfAuthorized {
                 return(0, 0)
             }
 
-            let calldataPtr := allocate(calldatasize())
-            calldatacopy(calldataPtr, 0, calldatasize())
+            let dataSize := calldatasize()
+            let ptr := mload(0x40)
+            calldatacopy(ptr, 0, dataSize)
 
             // 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
-            let senderPtr := allocate(20)
-            mstore(senderPtr, shl(96, caller()))
+            mstore(add(ptr, dataSize), shl(96, caller()))
 
             // Add 20 bytes for the address appended add the end
-            let success := call(gas(), handler, 0, calldataPtr, add(calldatasize(), 20), 0, 0)
+            let success := call(gas(), handler, 0, ptr, add(dataSize, 20), 0, 0)
 
-            let returnDataPtr := allocate(returndatasize())
-            returndatacopy(returnDataPtr, 0, returndatasize())
+            dataSize := returndatasize()
+            returndatacopy(ptr, 0, dataSize)
             if iszero(success) {
-                revert(returnDataPtr, returndatasize())
+                revert(ptr, dataSize)
             }
-            return(returnDataPtr, returndatasize())
+            return(ptr, dataSize)
         }
         /* solhint-enable no-inline-assembly */
     }

diff --git a/contracts/handler/CompatibilityFallbackHandler.sol b/contracts/handler/CompatibilityFallbackHandler.sol
@@ -97,23 +97,23 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
         assembly {
-            let internalCalldata := mload(0x40)
+            let ptr := mload(0x40)
             /**
              * Store `simulateAndRevert.selector`.
              * String representation is used to force right padding
              */
-            mstore(internalCalldata, "\xb4\xfa\xba\x09")
+            mstore(ptr, "\xb4\xfa\xba\x09")
+
+            let dataSize := calldatasize()
             /**
              * Abuse the fact that both this and the internal methods have the
              * same signature, and differ only in symbol name (and therefore,
              * selector) and copy calldata directly. This saves us approximately
              * 250 bytes of code and 300 gas at runtime over the
              * `abi.encodeWithSelector` builtin.
              */
-            calldatacopy(add(internalCalldata, 0x04), 0x04, sub(calldatasize(), 0x04))
+            calldatacopy(add(ptr, 0x04), 0x04, sub(dataSize, 0x04))
 
-            let returnPtr := mload(0x40)
-            mstore(0x40, add(returnPtr, 0x20))
             /**
              * `pop` is required here by the compiler, as top level expressions
              * can't have return values in inline assembly. `call` typically
@@ -126,15 +126,15 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
                     // address() has been changed to caller() to use the implementation of the Safe
                     caller(),
                     0,
-                    internalCalldata,
-                    calldatasize(),
+                    ptr,
+                    dataSize,
                     /**
                      * The `simulateAndRevert` call always reverts, and
                      * instead encodes whether or not it was successful in the return
                      * data. The first 32-byte word of the return data contains the
                      * `success` value, so write it to `returnPtr`.
                      */
-                    returnPtr,
+                    add(ptr, dataSize),
                     0x20
                 )
             )
@@ -151,7 +151,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
             mstore(0x40, add(response, responseSize))
             returndatacopy(response, 0x20, responseSize)
 
-            if iszero(mload(returnPtr)) {
+            if iszero(mload(add(ptr, dataSize))) {
                 revert(add(response, 0x20), mload(response))
             }
         }

diff --git a/contracts/libraries/MultiSend.sol b/contracts/libraries/MultiSend.sol
@@ -65,7 +65,6 @@ contract MultiSend {
                 if eq(success, 0) {
                     let errorLength := returndatasize()
                     let returnPtr := mload(0x40)
-                    mstore(0x40, add(returnPtr, returndatasize()))
                     returndatacopy(returnPtr, 0, errorLength)
                     revert(returnPtr, errorLength)
                 }

diff --git a/contracts/libraries/MultiSendCallOnly.sol b/contracts/libraries/MultiSendCallOnly.sol
@@ -60,7 +60,6 @@ contract MultiSendCallOnly {
                 if eq(success, 0) {
                     let errorLength := returndatasize()
                     let returnPtr := mload(0x40)
-                    mstore(0x40, add(returnPtr, returndatasize()))
                     returndatacopy(returnPtr, 0, errorLength)
                     revert(returnPtr, errorLength)
                 }

diff --git a/contracts/proxies/SafeProxy.sol b/contracts/proxies/SafeProxy.sol
@@ -41,12 +41,9 @@ contract SafeProxy {
                 mstore(ptr, _singleton)
                 return(ptr, 0x20)
             }
-            mstore(0x40, add(ptr, calldatasize()))
             calldatacopy(ptr, 0, calldatasize())
 
             let success := delegatecall(gas(), _singleton, ptr, calldatasize(), 0, 0)
-            ptr := mload(0x40)
-            mstore(0x40, add(ptr, returndatasize()))
             returndatacopy(ptr, 0, returndatasize())
             if eq(success, 0) {
                 revert(ptr, returndatasize())

diff --git a/contracts/proxies/SafeProxyFactory.sol b/contracts/proxies/SafeProxyFactory.sol
@@ -40,11 +40,7 @@ contract SafeProxyFactory {
             /// @solidity memory-safe-assembly
             assembly {
                 if eq(call(gas(), proxy, 0, add(initializer, 0x20), mload(initializer), 0, 0), 0) {
-                    let returnPtr := mload(0x40)
-                    let length := returndatasize()
-                    mstore(0x40, add(returnPtr, length))
-                    returndatacopy(returnPtr, 0, length)
-                    revert(returnPtr, length)
+                    revert(0, 0)
                 }
             }
             /* solhint-enable no-inline-assembly */

diff --git a/contracts/test/DelegateCaller.sol b/contracts/test/DelegateCaller.sol
@@ -18,7 +18,6 @@ contract DelegateCaller {
             assembly {
                 let ptr := mload(0x40)
                 let length := returndatasize()
-                mstore(0x40, add(ptr, length))
                 returndatacopy(ptr, 0, length)
                 revert(ptr, length)
             }