Add slow parsing detection to EOF layout fuzzing (#7516)

* Add slow parsing validation

Add CLI flags and fuzzing logic to enable "slow" parsing to be a
loggable error.

* picocli final field issue

* fix some array boundary issues in pretty print and testing

Signed-off-by: Danno Ferrin <danno@numisight.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Danno Ferrin <danno@numisight.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/CodeValidateSubCommand.java

@@ -126,9 +126,13 @@ public class CodeValidateSubCommand implements Runnable {
   private void checkCodeFromBufferedReader(final BufferedReader in) {
     try {
       for (String code = in.readLine(); code != null; code = in.readLine()) {
-        String validation = considerCode(code);
-        if (!Strings.isBlank(validation)) {
-          parentCommand.out.println(validation);
+        try {
+          String validation = considerCode(code);
+          if (!Strings.isBlank(validation)) {
+            parentCommand.out.println(validation);
+          }
+        } catch (RuntimeException e) {
+          parentCommand.out.println("fail: " + e.getMessage());
         }
       }
     } catch (IOException e) {
@@ -151,14 +155,17 @@ public class CodeValidateSubCommand implements Runnable {
   public String considerCode(final String hexCode) {
     Bytes codeBytes;
     try {
-      codeBytes =
-          Bytes.fromHexString(
-              hexCode.replaceAll("(^|\n)#[^\n]*($|\n)", "").replaceAll("[^0-9A-Za-z]", ""));
+      String strippedString =
+          hexCode.replaceAll("(^|\n)#[^\n]*($|\n)", "").replaceAll("[^0-9A-Za-z]", "");
+      if (Strings.isEmpty(strippedString)) {
+        return "";
+      }
+      codeBytes = Bytes.fromHexString(strippedString);
     } catch (RuntimeException re) {
       return "err: hex string -" + re;
     }
     if (codeBytes.isEmpty()) {
-      return "";
+      return "err: empty container";
     }
 
     EOFLayout layout = evm.get().parseEOF(codeBytes);

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/PrettyPrintSubCommand.java

@@ -89,7 +89,13 @@ public class PrettyPrintSubCommand implements Runnable {
     LogConfigurator.setLevel("", "OFF");
 
     for (var hexCode : codeList) {
-      Bytes container = Bytes.fromHexString(hexCode);
+      Bytes container;
+      try {
+        container = Bytes.fromHexString(hexCode);
+      } catch (IllegalArgumentException e) {
+        parentCommand.out.println("Invalid hex string: " + e.getMessage());
+        continue;
+      }
       if (container.get(0) != ((byte) 0xef) && container.get(1) != 0) {
         parentCommand.out.println(
             "Pretty printing of legacy EVM is not supported. Patches welcome!");

evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java

@@ -740,35 +740,59 @@ public record EOFLayout(
         OpcodeInfo ci = V1_OPCODES[byteCode[pc] & 0xff];
 
         if (ci.opcode() == RelativeJumpVectorOperation.OPCODE) {
-          int tableSize = byteCode[pc + 1] & 0xff;
-          out.printf("%02x%02x", byteCode[pc], byteCode[pc + 1]);
-          for (int j = 0; j <= tableSize; j++) {
-            out.printf("%02x%02x", byteCode[pc + j * 2 + 2], byteCode[pc + j * 2 + 3]);
-          }
-          out.printf(" # [%d] %s(", pc, ci.name());
-          for (int j = 0; j <= tableSize; j++) {
-            if (j != 0) {
-              out.print(',');
+          if (byteCode.length <= pc + 1) {
+            out.printf(
+                "    %02x # [%d] %s(<truncated instruction>)%n", byteCode[pc], pc, ci.name());
+            pc++;
+          } else {
+            int tableSize = byteCode[pc + 1] & 0xff;
+            out.printf("%02x%02x", byteCode[pc], byteCode[pc + 1]);
+            int calculatedTableEnd = pc + tableSize * 2 + 4;
+            int lastTableEntry = Math.min(byteCode.length, calculatedTableEnd);
+            for (int j = pc + 2; j < lastTableEntry; j++) {
+              out.printf("%02x", byteCode[j]);
+            }
+            out.printf(" # [%d] %s(", pc, ci.name());
+            for (int j = pc + 3; j < lastTableEntry; j += 2) {
+              // j indexes to the second byte of the word, to handle mid-word truncation
+              if (j != pc + 3) {
+                out.print(',');
+              }
+              int b0 = byteCode[j - 1]; // we want the sign extension, so no `& 0xff`
+              int b1 = byteCode[j] & 0xff;
+              out.print(b0 << 8 | b1);
+            }
+            if (byteCode.length < calculatedTableEnd) {
+              out.print("<truncated immediate>");
             }
-            int b0 = byteCode[pc + j * 2 + 2]; // we want the sign extension, so no `& 0xff`
-            int b1 = byteCode[pc + j * 2 + 3] & 0xff;
-            out.print(b0 << 8 | b1);
+            pc += tableSize * 2 + 4;
+            out.print(")\n");
           }
-          pc += tableSize * 2 + 4;
-          out.print(")\n");
         } else if (ci.opcode() == RelativeJumpOperation.OPCODE
             || ci.opcode() == RelativeJumpIfOperation.OPCODE) {
-          int b0 = byteCode[pc + 1] & 0xff;
-          int b1 = byteCode[pc + 2] & 0xff;
-          short delta = (short) (b0 << 8 | b1);
-          out.printf("%02x%02x%02x # [%d] %s(%d)", byteCode[pc], b0, b1, pc, ci.name(), delta);
+          if (pc + 1 >= byteCode.length) {
+            out.printf("    %02x # [%d] %s(<truncated immediate>)", byteCode[pc], pc, ci.name());
+          } else if (pc + 2 >= byteCode.length) {
+            out.printf(
+                "  %02x%02x # [%d] %s(<truncated immediate>)",
+                byteCode[pc], byteCode[pc + 1], pc, ci.name());
+          } else {
+            int b0 = byteCode[pc + 1] & 0xff;
+            int b1 = byteCode[pc + 2] & 0xff;
+            short delta = (short) (b0 << 8 | b1);
+            out.printf("%02x%02x%02x # [%d] %s(%d)", byteCode[pc], b0, b1, pc, ci.name(), delta);
+          }
           pc += 3;
           out.printf("%n");
         } else if (ci.opcode() == ExchangeOperation.OPCODE) {
-          int imm = byteCode[pc + 1] & 0xff;
-          out.printf(
-              "  %02x%02x # [%d] %s(%d, %d)",
-              byteCode[pc], imm, pc, ci.name(), imm >> 4, imm & 0x0F);
+          if (pc + 1 >= byteCode.length) {
+            out.printf("    %02x # [%d] %s(<truncated immediate>)", byteCode[pc], pc, ci.name());
+          } else {
+            int imm = byteCode[pc + 1] & 0xff;
+            out.printf(
+                "  %02x%02x # [%d] %s(%d, %d)",
+                byteCode[pc], imm, pc, ci.name(), imm >> 4, imm & 0x0F);
+          }
           pc += 2;
           out.printf("%n");
         } else {
@@ -784,7 +808,11 @@ public record EOFLayout(
           }
           out.printf(" # [%d] %s", pc, ci.name());
           if (advance == 2) {
-            out.printf("(%d)", byteCode[pc + 1] & 0xff);
+            if (byteCode.length <= pc + 1) {
+              out.print("(<truncated immediate>)");
+            } else {
+              out.printf("(%d)", byteCode[pc + 1] & 0xff);
+            }
           } else if (advance > 2) {
             out.print("(0x");
             for (int j = 1; j < advance && (pc + j) < byteCode.length; j++) {

testfuzz/build.gradle

@@ -39,6 +39,7 @@ dependencies {
 
   implementation 'com.fasterxml.jackson.core:jackson-databind'
   implementation 'com.gitlab.javafuzz:core'
+  implementation 'com.google.guava:guava'
   implementation 'info.picocli:picocli'
   implementation 'io.tmio:tuweni-bytes'
   implementation 'org.jacoco:org.jacoco.agent'
@@ -56,6 +57,7 @@ application {
 def corpusDir = "${buildDir}/generated/corpus"
 
 tasks.register("runFuzzer", JavaExec) {
+  doNotTrackState("Produces no artifacts")
   classpath = sourceSets.main.runtimeClasspath
   mainClass = 'org.hyperledger.besu.testfuzz.BesuFuzz'
 
@@ -69,6 +71,15 @@ tasks.register("runFuzzer", JavaExec) {
   }
 }
 
+// This fuzzes besu as an external client.  Besu fuzzing as a local client is enabled by default.
+tasks.register("fuzzBesu") {
+  dependsOn(":installDist")
+  doLast {
+    runFuzzer.args += "--client=besu=../build/install/besu/bin/evmtool code-validate"
+  }
+  finalizedBy("runFuzzer")
+}
+
 tasks.register("fuzzEvmone") {
   doLast {
     runFuzzer.args += "--client=evm1=evmone-eofparse"

testfuzz/src/main/java/org/hyperledger/besu/testfuzz/EofContainerSubCommand.java

@@ -17,14 +17,10 @@ package org.hyperledger.besu.testfuzz;
 import static org.hyperledger.besu.testfuzz.EofContainerSubCommand.COMMAND_NAME;
 
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.referencetests.EOFTestCaseSpec;
-import org.hyperledger.besu.evm.Code;
 import org.hyperledger.besu.evm.EVM;
 import org.hyperledger.besu.evm.MainnetEVMs;
-import org.hyperledger.besu.evm.code.CodeInvalid;
-import org.hyperledger.besu.evm.code.CodeV1;
-import org.hyperledger.besu.evm.code.EOFLayout;
-import org.hyperledger.besu.evm.code.EOFLayout.EOFContainerMode;
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 
 import java.io.File;
@@ -39,6 +35,9 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.core.JsonParser.Feature;
 import com.fasterxml.jackson.core.util.DefaultIndenter;
@@ -50,6 +49,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
 import com.gitlab.javafuzz.core.AbstractFuzzTarget;
+import com.google.common.base.Stopwatch;
 import org.apache.tuweni.bytes.Bytes;
 import picocli.CommandLine;
 import picocli.CommandLine.Option;
@@ -83,6 +83,23 @@ public class EofContainerSubCommand extends AbstractFuzzTarget implements Runnab
       description = "Add a client for differential fuzzing")
   private final Map<String, String> clients = new LinkedHashMap<>();
 
+  @Option(
+      names = {"--no-local-client"},
+      description = "Don't include built-in Besu with fuzzing")
+  private final Boolean noLocalClient = false;
+
+  @Option(
+      names = {"--time-limit-ns"},
+      defaultValue = "5000",
+      description = "Time threshold, in nanoseconds, that results in a fuzz error if exceeded")
+  private long timeThresholdMicros = 5_000;
+
+  @Option(
+      names = {"--time-limit-warmup"},
+      defaultValue = "2000",
+      description = "Minimum number of fuzz tests before a time limit fuzz error can occur")
+  private long timeThresholdIterations = 2_000;
+
   @CommandLine.ParentCommand private final BesuFuzzCommand parentCommand;
 
   static final ObjectMapper eofTestMapper = createObjectMapper();
@@ -91,7 +108,7 @@ public class EofContainerSubCommand extends AbstractFuzzTarget implements Runnab
           .getTypeFactory()
           .constructParametricType(Map.class, String.class, EOFTestCaseSpec.class);
 
-  List<ExternalClient> externalClients = new ArrayList<>();
+  List<FuzzingClient> fuzzingClients = new ArrayList<>();
   EVM evm = MainnetEVMs.pragueEOF(EvmConfiguration.DEFAULT);
   long validContainers;
   long totalContainers;
@@ -150,7 +167,10 @@ public class EofContainerSubCommand extends AbstractFuzzTarget implements Runnab
       }
     }
 
-    clients.forEach((k, v) -> externalClients.add(new StreamingClient(k, v.split(" "))));
+    if (!noLocalClient) {
+      fuzzingClients.add(new InternalClient("this"));
+    }
+    clients.forEach((k, v) -> fuzzingClients.add(new StreamingClient(k, v.split(" "))));
     System.out.println("Fuzzing client set: " + clients.keySet());
 
     try {
@@ -196,55 +216,54 @@ public class EofContainerSubCommand extends AbstractFuzzTarget implements Runnab
   public void fuzz(final byte[] bytes) {
     Bytes eofUnderTest = Bytes.wrap(bytes);
     String eofUnderTestHexString = eofUnderTest.toHexString();
-    Code code = evm.getCodeUncached(eofUnderTest);
-    Map<String, String> results = new LinkedHashMap<>();
-    boolean mismatch = false;
-    for (var client : externalClients) {
-      String value = client.differentialFuzz(eofUnderTestHexString);
-      results.put(client.getName(), value);
-      if (value == null || value.startsWith("fail: ")) {
-        mismatch = true; // if an external client fails, always report it as an error
-      }
-    }
-    boolean besuValid = false;
-    String besuReason;
-    if (!code.isValid()) {
-      besuReason = ((CodeInvalid) code).getInvalidReason();
-    } else if (code.getEofVersion() != 1) {
-      EOFLayout layout = EOFLayout.parseEOF(eofUnderTest);
-      if (layout.isValid()) {
-        besuReason = "Besu Parsing Error";
-        parentCommand.out.println(layout.version());
-        parentCommand.out.println(layout.invalidReason());
-        parentCommand.out.println(code.getEofVersion());
-        parentCommand.out.println(code.getClass().getName());
-        System.exit(1);
-        mismatch = true;
-      } else {
-        besuReason = layout.invalidReason();
-      }
-    } else if (EOFContainerMode.INITCODE.equals(
-        ((CodeV1) code).getEofLayout().containerMode().get())) {
-      besuReason = "Code is initcode, not runtime";
-    } else {
-      besuReason = "OK";
-      besuValid = true;
-    }
-    for (var entry : results.entrySet()) {
-      mismatch =
-          mismatch
-              || besuValid != entry.getValue().toUpperCase(Locale.getDefault()).startsWith("OK");
-    }
-    if (mismatch) {
-      parentCommand.out.println("besu: " + besuReason);
-      for (var entry : results.entrySet()) {
+
+    AtomicBoolean passHappened = new AtomicBoolean(false);
+    AtomicBoolean failHappened = new AtomicBoolean(false);
+
+    Map<String, String> resultMap =
+        fuzzingClients.stream()
+            .parallel()
+            .map(
+                client -> {
+                  Stopwatch stopwatch = Stopwatch.createStarted();
+                  String value = client.differentialFuzz(eofUnderTestHexString);
+                  stopwatch.stop();
+                  long elapsedMicros = stopwatch.elapsed(TimeUnit.MICROSECONDS);
+                  if (elapsedMicros > timeThresholdMicros
+                      && totalContainers > timeThresholdIterations) {
+                    Hash name = Hash.hash(eofUnderTest);
+                    parentCommand.out.printf(
+                        "%s: slow validation %d µs%n", client.getName(), elapsedMicros);
+                    try {
+                      Files.writeString(
+                          Path.of("slow-" + client.getName() + "-" + name + ".hex"),
+                          eofUnderTestHexString);
+                    } catch (IOException e) {
+                      throw new RuntimeException(e);
+                    }
+                  }
+                  if (value.toLowerCase(Locale.ROOT).startsWith("ok")) {
+                    passHappened.set(true);
+                  } else if (value.toLowerCase(Locale.ROOT).startsWith("err")) {
+                    failHappened.set(true);
+                  } else {
+                    // unexpected output: trigger a mismatch
+                    passHappened.set(true);
+                    failHappened.set(true);
+                  }
+                  return Map.entry(client.getName(), value);
+                })
+            .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+    if (passHappened.get() && failHappened.get()) {
+      for (var entry : resultMap.entrySet()) {
         parentCommand.out.println(entry.getKey() + ": " + entry.getValue());
       }
       parentCommand.out.println("code: " + eofUnderTest.toUnprefixedHexString());
       parentCommand.out.println("size: " + eofUnderTest.size());
       parentCommand.out.println();
     } else {
-      if (besuValid) {
+      if (passHappened.get()) {
         validContainers++;
       }
       totalContainers++;

testfuzz/src/main/java/org/hyperledger/besu/testfuzz/FuzzingClient.java

@@ -14,7 +14,7 @@
  */
 package org.hyperledger.besu.testfuzz;
 
-interface ExternalClient {
+interface FuzzingClient {
 
   String getName();
 

testfuzz/src/main/java/org/hyperledger/besu/testfuzz/InternalClient.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.testfuzz;
+
+import org.hyperledger.besu.evm.Code;
+import org.hyperledger.besu.evm.EVM;
+import org.hyperledger.besu.evm.MainnetEVMs;
+import org.hyperledger.besu.evm.code.CodeInvalid;
+import org.hyperledger.besu.evm.code.CodeV1;
+import org.hyperledger.besu.evm.code.EOFLayout;
+import org.hyperledger.besu.evm.code.EOFLayout.EOFContainerMode;
+import org.hyperledger.besu.evm.internal.EvmConfiguration;
+
+import org.apache.tuweni.bytes.Bytes;
+
+class InternalClient implements FuzzingClient {
+  String name;
+  final EVM evm;
+
+  public InternalClient(final String clientName) {
+    this.name = clientName;
+    this.evm = MainnetEVMs.pragueEOF(EvmConfiguration.DEFAULT);
+  }
+
+  @Override
+  public String getName() {
+    return name;
+  }
+
+  @Override
+  @SuppressWarnings("java:S2142")
+  public String differentialFuzz(final String data) {
+    try {
+      Bytes clientData = Bytes.fromHexString(data);
+      Code code = evm.getCodeUncached(clientData);
+      if (code.getEofVersion() < 1) {
+        return "err: legacy EVM";
+      } else if (!code.isValid()) {
+        return "err: " + ((CodeInvalid) code).getInvalidReason();
+      } else {
+        EOFLayout layout = ((CodeV1) code).getEofLayout();
+        if (EOFContainerMode.INITCODE.equals(layout.containerMode().get())) {
+          return "err: initcode container when runtime mode expected";
+        }
+        return "OK %d/%d/%d"
+            .formatted(
+                layout.getCodeSectionCount(), layout.getSubcontainerCount(), layout.dataLength());
+      }
+    } catch (RuntimeException e) {
+      return "fail: " + e.getMessage();
+    }
+  }
+}

testfuzz/src/main/java/org/hyperledger/besu/testfuzz/SingleQueryClient.java

@@ -24,7 +24,7 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 @SuppressWarnings({"java:S106", "CallToPrintStackTrace"}) // we use lots the console, on purpose
-class SingleQueryClient implements ExternalClient {
+class SingleQueryClient implements FuzzingClient {
   final String name;
   String[] command;
   Pattern okRegexp;

testfuzz/src/main/java/org/hyperledger/besu/testfuzz/StreamingClient.java

@@ -19,7 +19,7 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.nio.charset.StandardCharsets;
 
-class StreamingClient implements ExternalClient {
+class StreamingClient implements FuzzingClient {
   final String name;
   final BufferedReader reader;
   final PrintWriter writer;