Address feedback

ethereumjs · Mar 14, 2022 · 80933d2 · 80933d2
1 parent 3fb6bab
commit 80933d2
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 51 deletions.
diff --git a/packages/vm/src/evm/interpreter.ts b/packages/vm/src/evm/interpreter.ts
@@ -94,41 +94,44 @@ export default class Interpreter {
 
   async run(code: Buffer, opts: InterpreterOpts = {}): Promise<InterpreterResult> {
     if (
-      this._vm._common.isActivatedEIP(3540) &&
-      code.slice(0, 1).equals(Buffer.from('ef', 'hex'))
+      !this._vm._common.isActivatedEIP(3540) ||
+      !code.slice(0, 1).equals(Buffer.from('ef', 'hex'))
     ) {
-      if (code.slice(1, 2).equals(Buffer.from('00', 'hex'))) {
-        if (code.slice(2, 3).equals(Buffer.from('01', 'hex'))) {
-          // Code is EOF1 format
-          const codeSections = eof1CodeAnalysis(code)
-          if (!codeSections) {
-            return {
-              runState: this._runState,
-              exceptionError: new VmError(ERROR.INVALID_EOF_FORMAT),
-            }
-          }
-          // Set code to code section which starts at byte position 7 if code only or 10 if data section is present
-          if (codeSections!.data) {
-            this._runState.code = code.slice(10, 10 + codeSections!.code)
-          } else {
-            this._runState.code = code.slice(7, 7 + codeSections!.code)
-          }
-        } else {
-          return {
-            runState: this._runState,
-            exceptionError: new VmError(ERROR.INVALID_EOF_FORMAT),
-          }
-        }
-      } else {
+      // EIP-3540 isn't active and first byte is not 0xEF - treat as legacy bytecode
+      this._runState.code = code
+    } else if (this._vm._common.isActivatedEIP(3540)) {
+      if (!code.slice(1, 2).equals(Buffer.from('00', 'hex'))) {
+        // Bytecode contains invalid EOF magic byte
         return {
           runState: this._runState,
           exceptionError: new VmError(ERROR.INVALID_BYTECODE_RESULT),
         }
       }
-    } else {
-      this._runState.code = code
-    }
+      if (!code.slice(2, 3).equals(Buffer.from('01', 'hex'))) {
+        // Bytecode contains invalid EOF version number
+        return {
+          runState: this._runState,
+          exceptionError: new VmError(ERROR.INVALID_EOF_FORMAT),
+        }
+      }
+      // Code is EOF1 format
+      const codeSections = eof1CodeAnalysis(code)
+      if (!codeSections) {
+        // Code is invalid EOF1 format if `codeSections` is falsy
+        return {
+          runState: this._runState,
+          exceptionError: new VmError(ERROR.INVALID_EOF_FORMAT),
+        }
+      }
 
+      if (codeSections.data) {
+        // Set code to EOF container code section which starts at byte position 10 if data section is present
+        this._runState.code = code.slice(10, 10 + codeSections!.code)
+      } else {
+        // Set code to EOF container code section which starts at byte position 7 if no data section is present
+        this._runState.code = code.slice(7, 7 + codeSections!.code)
+      }
+    }
     this._runState.programCounter = opts.pc ?? this._runState.programCounter
     // Check that the programCounter is in range
     const pc = this._runState.programCounter

diff --git a/packages/vm/src/evm/opcodes/util.ts b/packages/vm/src/evm/opcodes/util.ts
@@ -263,6 +263,14 @@ export function updateSstoreGas(
   }
 }
 
+/**
+ *
+ * @param container A `Buffer` containing bytecode to be checked for EOF1 compliance
+ * @returns an object containing the size of the code section and data sections for a valid
+ * EOF1 container or else undefined if `container` is not valid EOF1 bytecode
+ *
+ * Note: See https://eips.ethereum.org/EIPS/eip-3540 for further details
+ */
 export const eof1CodeAnalysis = (container: Buffer) => {
   const magic = 0x00
   const version = 0x01
@@ -274,30 +282,45 @@ export const eof1CodeAnalysis = (container: Buffer) => {
     code: 0,
     data: 0,
   }
-  if (container[1] === magic && container[2] === version) {
-    if (container.length > 7 && container[3] === secCode && container[6] === secTerminator) {
-      sectionSizes.code = (container[4] << 8) | container[5]
-      computedContainerSize = 7 + sectionSizes.code
-      // Code size cannot be 0
-      if (sectionSizes.code < 1) return
-    } else if (
-      container.length > 10 &&
-      container[3] === secCode &&
-      container[6] === secData &&
-      container[9] === secTerminator
-    ) {
-      sectionSizes.code = (container[4] << 8) | container[5]
-      sectionSizes.data = (container[7] << 8) | container[8]
-      computedContainerSize = 10 + sectionSizes.code + sectionSizes.data
-      // Code & Data sizes cannot be 0
-      if (sectionSizes.code < 1 || sectionSizes.data < 1) return
-    }
-    if (container.length !== computedContainerSize) {
-      // Scanned code does not match length of contract byte code
-      return
-    }
-    return sectionSizes
+  if (container[1] !== magic || container[2] !== version)
+    // Bytecode does not contain EOF1 "magic" or version number in expected positions
+    return
+
+  if (
+    // EOF1 bytecode must be more than 7 bytes long for EOF1 header plus code section (but no data section)
+    container.length > 7 &&
+    // EOF1 code section indicator
+    container[3] === secCode &&
+    // EOF1 header terminator
+    container[6] === secTerminator
+  ) {
+    sectionSizes.code = (container[4] << 8) | container[5]
+    // Calculate expected length of EOF1 container based on code section
+    computedContainerSize = 7 + sectionSizes.code
+    // EOF1 code section must be at least 1 byte long
+    if (sectionSizes.code < 1) return
+  } else if (
+    // EOF1 container must be more than 10 bytes long if data section is included
+    container.length > 10 &&
+    // EOF1 code section indicator
+    container[3] === secCode &&
+    // EOF1 data section indicator
+    container[6] === secData &&
+    // EOF1 header terminator
+    container[9] === secTerminator
+  ) {
+    sectionSizes.code = (container[4] << 8) | container[5]
+    sectionSizes.data = (container[7] << 8) | container[8]
+    // Calculate expected length of EOF1 container based on code and data sections
+    computedContainerSize = 10 + sectionSizes.code + sectionSizes.data
+    // Code & Data sizes cannot be 0
+    if (sectionSizes.code < 1 || sectionSizes.data < 1) return
+  }
+  if (container.length !== computedContainerSize) {
+    // Computed container length based on section details does not match length of actual bytecode
+    return
   }
+  return sectionSizes
 }
 
 export const eof1ValidOpcodes = (code: Buffer) => {