fix(EVM): Do not check memory boundaries if size is zero (#1114)

matter-labs · Dec 5, 2024 · 1ab2755 · 1ab2755
1 parent 6c115b4
commit 1ab2755
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 45 deletions.
diff --git a/system-contracts/contracts/EvmEmulator.yul b/system-contracts/contracts/EvmEmulator.yul
@@ -280,10 +280,12 @@ object "EvmEmulator" {
         }
 
         function checkMemIsAccessible(relativeOffset, size) {
-            checkOverflow(relativeOffset, size)
+            if size {
+                checkOverflow(relativeOffset, size)
 
-            if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
-                panic()
+                if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
+                    panic()
+                }   
             }
         }
 
@@ -2676,14 +2678,17 @@ object "EvmEmulator" {
                     offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
                     size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
 
-                    checkMemIsAccessible(offset, size)
+                    if size {
+                        checkMemIsAccessible(offset, size)
 
-                    evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+                        evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+
+                        returnLen := size
+
+                        // Don't check overflow here since previous checks are enough to ensure this is safe
+                        returnOffset := add(MEM_OFFSET(), offset)
+                    }
 
-                    returnLen := size
-
-                    // Don't check overflow here since previous checks are enough to ensure this is safe
-                    returnOffset := add(MEM_OFFSET(), offset)
                     break
                 }
                 case 0xF4 { // OP_DELEGATECALL
@@ -2710,12 +2715,19 @@ object "EvmEmulator" {
                     popStackCheck(sp, 2)
                     offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
                     size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
-
-                    checkMemIsAccessible(offset, size)
-                    evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
-
-                    // Don't check overflow here since previous checks are enough to ensure this is safe
-                    offset := add(offset, MEM_OFFSET())
+
+                    switch iszero(size)
+                    case 0 {
+                        checkMemIsAccessible(offset, size)
+                        evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+
+                        // Don't check overflow here since previous checks are enough to ensure this is safe
+                        offset := add(offset, MEM_OFFSET())
+                    }
+                    default {
+                        offset := MEM_OFFSET()
+                    }
+
 
                     if eq(isCallerEVM, 1) {
                         offset := sub(offset, 32)
@@ -3340,10 +3352,12 @@ object "EvmEmulator" {
             }
 
             function checkMemIsAccessible(relativeOffset, size) {
-                checkOverflow(relativeOffset, size)
+                if size {
+                    checkOverflow(relativeOffset, size)
 
-                if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
-                    panic()
+                    if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
+                        panic()
+                    }   
                 }
             }
 
@@ -5736,14 +5750,17 @@ object "EvmEmulator" {
                         offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
                         size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
 
-                        checkMemIsAccessible(offset, size)
+                        if size {
+                            checkMemIsAccessible(offset, size)
 
-                        evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+                            evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+
+                            returnLen := size
+
+                            // Don't check overflow here since previous checks are enough to ensure this is safe
+                            returnOffset := add(MEM_OFFSET(), offset)
+                        }
 
-                        returnLen := size
-
-                        // Don't check overflow here since previous checks are enough to ensure this is safe
-                        returnOffset := add(MEM_OFFSET(), offset)
                         break
                     }
                     case 0xF4 { // OP_DELEGATECALL
@@ -5770,12 +5787,19 @@ object "EvmEmulator" {
                         popStackCheck(sp, 2)
                         offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
                         size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
-
-                        checkMemIsAccessible(offset, size)
-                        evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
-
-                        // Don't check overflow here since previous checks are enough to ensure this is safe
-                        offset := add(offset, MEM_OFFSET())
+
+                        switch iszero(size)
+                        case 0 {
+                            checkMemIsAccessible(offset, size)
+                            evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+
+                            // Don't check overflow here since previous checks are enough to ensure this is safe
+                            offset := add(offset, MEM_OFFSET())
+                        }
+                        default {
+                            offset := MEM_OFFSET()
+                        }
+
 
                         if eq(isCallerEVM, 1) {
                             offset := sub(offset, 32)

diff --git a/system-contracts/evm-emulator/EvmEmulatorFunctions.template.yul b/system-contracts/evm-emulator/EvmEmulatorFunctions.template.yul
@@ -218,10 +218,12 @@ function expandMemory2(retOffset, retSize, argsOffset, argsSize) -> maxExpand {
 }
 
 function checkMemIsAccessible(relativeOffset, size) {
-    checkOverflow(relativeOffset, size)
+    if size {
+        checkOverflow(relativeOffset, size)
 
-    if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
-        panic()
+        if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
+            panic()
+        }   
     }
 }
 

diff --git a/system-contracts/evm-emulator/EvmEmulatorLoop.template.yul b/system-contracts/evm-emulator/EvmEmulatorLoop.template.yul
@@ -1454,14 +1454,17 @@ for { } true { } {
         offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
         size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
 
-        checkMemIsAccessible(offset, size)
+        if size {
+            checkMemIsAccessible(offset, size)
 
-        evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+            evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+
+            returnLen := size
+
+            // Don't check overflow here since previous checks are enough to ensure this is safe
+            returnOffset := add(MEM_OFFSET(), offset)
+        }
 
-        returnLen := size
-
-        // Don't check overflow here since previous checks are enough to ensure this is safe
-        returnOffset := add(MEM_OFFSET(), offset)
         break
     }
     case 0xF4 { // OP_DELEGATECALL
@@ -1488,12 +1491,19 @@ for { } true { } {
         popStackCheck(sp, 2)
         offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
         size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
-
-        checkMemIsAccessible(offset, size)
-        evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
-
-        // Don't check overflow here since previous checks are enough to ensure this is safe
-        offset := add(offset, MEM_OFFSET())
+
+        switch iszero(size)
+        case 0 {
+            checkMemIsAccessible(offset, size)
+            evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
+
+            // Don't check overflow here since previous checks are enough to ensure this is safe
+            offset := add(offset, MEM_OFFSET())
+        }
+        default {
+            offset := MEM_OFFSET()
+        }
+
 
         if eq(isCallerEVM, 1) {
             offset := sub(offset, 32)