From 2fe7273bb481d4d6e1b092900ae68684d2bca8fd Mon Sep 17 00:00:00 2001
From: Nicholas Rodrigues Lordello <nick@safe.global>
Date: Thu, 14 Dec 2023 10:50:57 +0100
Subject: [PATCH] Memory-Safe Proxy Code Without Allocation (#67)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR modifies the account fallback code to no longer use allocations,
as they are not required for memory-safety. [From the
docs](https://docs.soliditylang.org/en/v0.8.22/assembly.html#memory-safety):

> In particular, a memory-safe assembly block may only access the
following memory ranges:
>
> * [...]
> * Temporary memory that is located after the value of the free memory
pointer at the beginning of the assembly block, i.e. memory that is
“allocated” at the free memory pointer **without updating the free
memory pointer**.
>
> [...]
>
> On the other hand, **the following code _is_ memory safe**, because
memory beyond the location pointed to by the free memory pointer can
safely be used as temporary scratch space:
>
> ```solidity
> assembly ("memory-safe") {
>   let p := mload(0x40)
>   returndatacopy(p, 0, returndatasize())
>   revert(p, returndatasize())
> }
> ```

I also [did some
investigation](https://github.com/safe-global/safe-contracts/issues/544#issuecomment-1835746635)
and found that when variables are moved into memory, the space gets
reserved **before** user code starts, meaning that the free memory
pointer already accounts for the reserved space (i.e. setting variables
cannot write past the free memory pointer ever). With that in mind, we
do not need to update the free memory pointer when we write past it for
scratch space.

cc @mmv08
---
 accounts/README.md | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/accounts/README.md b/accounts/README.md
index fd1ab11..c3e5e8d 100644
--- a/accounts/README.md
+++ b/accounts/README.md
@@ -61,33 +61,26 @@ An example implementation of such a `fallback` function is:
         bytes32 slot = SAFE_CORE_PROTOCOL_MANAGER_SLOT;
         /// @solidity memory-safe-assembly
         assembly {
-            function allocate(length) -> pos {
-                pos := mload(0x40)
-                mstore(0x40, add(pos, length))
-            }
-
             let protocol := sload(slot)
             if iszero(protocol) {
                 return(0, 0)
             }
 
-            let calldataPtr := allocate(calldatasize())
-            calldatacopy(calldataPtr, 0, calldatasize())
+            let ptr := mload(0x40)
+            calldatacopy(ptr, 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
-            let senderPtr := allocate(20)
-            mstore(senderPtr, shl(96, caller()))
+            mstore(add(ptr, calldatasize()), shl(96, caller()))
 
             // Add 20 bytes for the address appended add the end
-            let success := call(gas(), protocol, 0, calldataPtr, add(calldatasize(), 20), 0, 0)
+            let success := call(gas(), protocol, 0, ptr, add(calldatasize(), 20), 0, 0)
 
-            let returnDataPtr := allocate(returndatasize())
-            returndatacopy(returnDataPtr, 0, returndatasize())
+            returndatacopy(ptr, 0, returndatasize())
             if iszero(success) {
-                revert(returnDataPtr, returndatasize())
+                revert(ptr, returndatasize())
             }
-            return(returnDataPtr, returndatasize())
+            return(ptr, returndatasize())
         }
     }
 ```