Reworked script engine detector

Check for containment of payload in script content in all overloads of
eval.

examples/BUILD.bazel

@@ -188,7 +188,6 @@ java_fuzz_target_test(
         "src/main/java/com/example/CommonsTextFuzzer.java",
     ],
     fuzzer_args = [
-        "-fork=8",
         "-use_value_profile=1",
     ],
     tags = ["manual"],

examples/src/main/java/com/example/CommonsTextFuzzer.java

@@ -1,4 +1,4 @@
-// Copyright 2022 Code Intelligence GmbH
+// Copyright 2023 Code Intelligence GmbH
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,13 +15,12 @@
 package com.example;
 
 import com.code_intelligence.jazzer.api.FuzzedDataProvider;
-import com.code_intelligence.jazzer.api.Jazzer;
 import org.apache.commons.text.StringSubstitutor;
 
 public class CommonsTextFuzzer {
   public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     try {
-      StringSubstitutor.createInterpolator().replace(data.consumeAsciiString(20));
+      StringSubstitutor.createInterpolator().replace(data.consumeString(20));
     } catch (
         java.lang.IllegalArgumentException | java.lang.ArrayIndexOutOfBoundsException ignored) {
     }

maven_install.json

@@ -1,7 +1,7 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": 25727711,
-  "__RESOLVED_ARTIFACTS_HASH": 1912979319,
+  "__INPUT_ARTIFACTS_HASH": 1679182184,
+  "__RESOLVED_ARTIFACTS_HASH": -1745128755,
   "conflict_resolution": {
     "junit:junit:4.12": "junit:junit:4.13.2"
   },
@@ -210,12 +210,24 @@
       },
       "version": "1.0-alpha2"
     },
+    "org.apache.commons:commons-lang3": {
+      "shasums": {
+        "jar": "4ee380259c068d1dbe9e84ab52186f2acd65de067ec09beff731fca1697fdb16"
+      },
+      "version": "3.11"
+    },
     "org.apache.commons:commons-math3": {
       "shasums": {
         "jar": "6268a9a0ea3e769fc493a21446664c0ef668e48c93d126791f6f3f757978fee2"
       },
       "version": "3.2"
     },
+    "org.apache.commons:commons-text": {
+      "shasums": {
+        "jar": "0812f284ac5dd0d617461d9a2ab6ac6811137f25122dfffd4788a4871e732d00"
+      },
+      "version": "1.9"
+    },
     "org.apache.logging.log4j:log4j-api": {
       "shasums": {
         "jar": "8caf58db006c609949a0068110395a33067a2bad707c3da35e959c0473f9a916"
@@ -596,6 +608,9 @@
     "junit:junit": [
       "org.hamcrest:hamcrest-core"
     ],
+    "org.apache.commons:commons-text": [
+      "org.apache.commons:commons-lang3"
+    ],
     "org.apache.logging.log4j:log4j-core": [
       "org.apache.logging.log4j:log4j-api"
     ],
@@ -1199,6 +1214,25 @@
       "org.apache.commons.imaging.internal",
       "org.apache.commons.imaging.palette"
     ],
+    "org.apache.commons:commons-lang3": [
+      "org.apache.commons.lang3",
+      "org.apache.commons.lang3.arch",
+      "org.apache.commons.lang3.builder",
+      "org.apache.commons.lang3.compare",
+      "org.apache.commons.lang3.concurrent",
+      "org.apache.commons.lang3.concurrent.locks",
+      "org.apache.commons.lang3.event",
+      "org.apache.commons.lang3.exception",
+      "org.apache.commons.lang3.function",
+      "org.apache.commons.lang3.math",
+      "org.apache.commons.lang3.mutable",
+      "org.apache.commons.lang3.reflect",
+      "org.apache.commons.lang3.stream",
+      "org.apache.commons.lang3.text",
+      "org.apache.commons.lang3.text.translate",
+      "org.apache.commons.lang3.time",
+      "org.apache.commons.lang3.tuple"
+    ],
     "org.apache.commons:commons-math3": [
       "org.apache.commons.math3",
       "org.apache.commons.math3.analysis",
@@ -1262,6 +1296,15 @@
       "org.apache.commons.math3.transform",
       "org.apache.commons.math3.util"
     ],
+    "org.apache.commons:commons-text": [
+      "org.apache.commons.text",
+      "org.apache.commons.text.diff",
+      "org.apache.commons.text.io",
+      "org.apache.commons.text.lookup",
+      "org.apache.commons.text.matcher",
+      "org.apache.commons.text.similarity",
+      "org.apache.commons.text.translate"
+    ],
     "org.apache.logging.log4j:log4j-api": [
       "org.apache.logging.log4j",
       "org.apache.logging.log4j.internal",
@@ -2029,7 +2072,9 @@
       "net.bytebuddy:byte-buddy-agent",
       "net.sf.jopt-simple:jopt-simple",
       "org.apache.commons:commons-imaging",
+      "org.apache.commons:commons-lang3",
       "org.apache.commons:commons-math3",
+      "org.apache.commons:commons-text",
       "org.apache.logging.log4j:log4j-api",
       "org.apache.logging.log4j:log4j-core",
       "org.apache.xmlgraphics:batik-anim",

sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/BUILD.bazel

@@ -32,6 +32,7 @@ java_library(
     name = "script_engine_injection",
     srcs = ["ScriptEngineInjection.java"],
     deps = [
+        "//sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/utils:reflection_utils",
         "//src/main/java/com/code_intelligence/jazzer/api:hooks",
     ],
 )
@@ -52,8 +53,8 @@ kt_jvm_library(
     visibility = ["//sanitizers:__pkg__"],
     runtime_deps = [
         ":regex_roadblocks",
-        ":server_side_request_forgery",
         ":script_engine_injection",
+        ":server_side_request_forgery",
         ":sql_injection",
     ],
     deps = [

sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/ScriptEngineInjection.java

@@ -1,4 +1,4 @@
-// Copyright 2022 Code Intelligence GmbH
+// Copyright 2023 Code Intelligence GmbH
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,111 +14,133 @@
 
 package com.code_intelligence.jazzer.sanitizers;
 
-import static java.util.Collections.unmodifiableSet;
-import static java.util.stream.Collectors.toSet;
+import static com.code_intelligence.jazzer.sanitizers.utils.ReflectionUtils.methodFromObject;
 
 import com.code_intelligence.jazzer.api.FuzzerSecurityIssueCritical;
 import com.code_intelligence.jazzer.api.HookType;
 import com.code_intelligence.jazzer.api.Jazzer;
 import com.code_intelligence.jazzer.api.MethodHook;
-import com.code_intelligence.jazzer.api.MethodHooks;
-import java.io.IOException;
+import java.io.BufferedReader;
 import java.io.Reader;
 import java.lang.invoke.MethodHandle;
-import java.util.Arrays;
-import java.util.Set;
-import java.util.stream.Stream;
-import javax.script.ScriptEngineManager;
+import java.lang.reflect.Method;
+import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
- * Detects Script Engine injection
+ * Detects Script Engine injections.
  *
  * <p>
  * The hooks in this class attempt to detect user input flowing into
- * {@link javax.script.ScriptEngine.eval} that might lead to
- * remote code executions depending on the scripting engine's capabilities.
- * Before JDK 15, the Nashorn Engine
- * was registered by default with ScriptEngineManager under several aliases,
- * including "js". Nashorn allows
+ * {@link javax.script.ScriptEngine#eval(String)} and the like that might lead
+ * to remote code executions depending on the scripting engine's capabilities.
+ * Before JDK 15, the Nashorn Engine was registered by default with
+ * ScriptEngineManager under several aliases, including "js". Nashorn allows
  * access to JVM classes, for example {@link java.lang.Runtime} allowing the
- * execution of arbitrary OS commands.
- * Several other scripting engines can be embedded to the JVM (they must follow
- * the <a href="https://www.jcp.org/en/jsr/detail?id=223">JSR-223</a>
+ * execution of arbitrary OS commands. Several other scripting engines can be
+ * embedded to the JVM (they must follow the
+ * <a href="https://www.jcp.org/en/jsr/detail?id=223">JSR-223 </a>
  * specification).
- * </p>
  **/
+@SuppressWarnings({"unused", "OptionalAssignedToNull", "OptionalUsedAsFieldOrParameterType"})
 public final class ScriptEngineInjection {
-  private static final String ENGINE = "js";
-  private static final String PAYLOAD = "1+1";
+  private static final String PAYLOAD = "\"jaz\"+\"zer\"";
 
-  private static char[] guideMarkableReaderTowardsEquality(Reader reader, String target, int id)
-      throws IOException {
-    final int size = target.length();
-    char[] current = new char[size];
-    int n = 0;
+  private static Optional<Method> eval;
+  private static Optional<Method> evalWithScriptContext;
+  private static Optional<Method> evalWithBindings;
 
-    if (!reader.markSupported()) {
-      throw new IllegalStateException("Reader does not support mark - not possible to fuzz");
-    }
-
-    reader.mark(size);
+  // Hook all eval variants and check for fuzzer input flowing into the script.
+  // The content of reader parameters is converted to a string and checked for
+  // containment of the payload. Afterwards, the string variant of eval is
+  // called manually. As the bootstrap class loader can not access javax
+  // classes anymore, the method handle is looked up lazily based on the passed
+  // in object. That's not thread safe, but ok in this context.
 
-    while (n < size) {
-      int count = reader.read(current, n, size - n);
-      if (count < 0)
-        break;
-      n += count;
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval", targetMethodDescriptor = "(Ljava/lang/String;)Ljava/lang/Object;")
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval", targetMethodDescriptor = "(Ljava/io/Reader;)Ljava/lang/Object;")
+  public static Object
+  checkScriptEngineExecute(MethodHandle method, Object thisObject, Object[] arguments, int hookId)
+      throws Throwable {
+    if (eval == null) {
+      eval = methodFromObject(thisObject, "eval", "java.lang.String");
+    }
+    if (eval.isPresent()) {
+      String content = checkScriptContent(arguments[0], hookId);
+      return eval.get().invoke(thisObject, content);
+    } else {
+      return method.invokeWithArguments(thisObject, arguments);
     }
-    reader.reset();
-
-    Jazzer.guideTowardsEquality(new String(current), target, id);
-
-    return current;
   }
 
-  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngineManager",
-      targetMethod = "registerEngineName")
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/lang/String;Ljavax/script/ScriptContext;)Ljava/lang/Object;")
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/io/Reader;Ljavax/script/ScriptContext;)Ljava/lang/Object;")
   public static Object
-  ensureScriptEngine(MethodHandle method, Object thisObject, Object[] arguments, int hookId)
-      throws Throwable {
-    return method.invokeWithArguments(Stream
-                                          .concat(Stream.of(thisObject),
-                                              Stream.concat(Stream.of((Object) ENGINE),
-                                                  Arrays.stream(arguments, 1, arguments.length)))
-                                          .toArray());
+  checkScriptEngineExecuteScriptContext(
+      MethodHandle method, Object thisObject, Object[] arguments, int hookId) throws Throwable {
+    if (evalWithScriptContext == null) {
+      evalWithScriptContext =
+          methodFromObject(thisObject, "eval", "java.lang.String", "javax.script.ScriptContext");
+    }
+    if (evalWithScriptContext.isPresent()) {
+      String content = checkScriptContent(arguments[0], hookId);
+      return evalWithScriptContext.get().invoke(thisObject, content, arguments[1]);
+    } else {
+      return method.invokeWithArguments(thisObject, arguments);
+    }
   }
 
-  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngineManager",
-      targetMethod = "getEngineByName",
-      targetMethodDescriptor = "(Ljava/lang/String;)Ljavax/script/ScriptEngine;")
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/lang/String;Ljavax/script/Bindings;)Ljava/lang/Object;")
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/io/Reader;Ljavax/script/Bindings;)Ljava/lang/Object;")
   public static Object
-  hookEngineName(MethodHandle method, Object thisObject, Object[] arguments, int hookId)
-      throws Throwable {
-    String engine = (String) arguments[0];
-    Jazzer.guideTowardsEquality(engine, ENGINE, hookId);
-    return method.invokeWithArguments(
-        Stream.concat(Stream.of(thisObject), Arrays.stream(arguments)).toArray());
+  checkScriptEngineExecuteBindings(
+      MethodHandle method, Object thisObject, Object[] arguments, int hookId) throws Throwable {
+    if (evalWithBindings == null) {
+      evalWithBindings =
+          methodFromObject(thisObject, "eval", "java.lang.String", "javax.script.Bindings");
+    }
+    if (evalWithBindings.isPresent()) {
+      String content = checkScriptContent(arguments[0], hookId);
+      return evalWithBindings.get().invoke(thisObject, content, arguments[1]);
+    } else {
+      return method.invokeWithArguments(thisObject, arguments);
+    }
   }
 
-  @MethodHook(type = HookType.BEFORE, targetClassName = "javax.script.ScriptEngine",
-      targetMethod = "eval", targetMethodDescriptor = "(Ljava/lang/String;)Ljava/lang/Object;")
-  @MethodHook(type = HookType.BEFORE, targetClassName = "javax.script.ScriptEngine",
-      targetMethod = "eval", targetMethodDescriptor = "(Ljava/io/Reader;)Ljava/lang/Object;")
-  public static void
-  checkScriptEngineExecute(MethodHandle method, Object thisObject, Object[] arguments, int hookId)
-      throws Throwable {
+  private static String checkScriptContent(Object argument, int hookId) {
     String script = null;
+    if (argument != null) {
+      if (argument instanceof String) {
+        script = (String) argument;
+      } else {
+        // This assumes that the reader content can safely be read completely
+        // and used as string parameter later on. Special reader
+        // implementations could be problematic, but are very unlikely to be
+        // used in this context.
+        script = readAll((Reader) argument);
+      }
 
-    if (arguments[0] instanceof String) {
-      script = (String) arguments[0];
-      Jazzer.guideTowardsEquality(script, PAYLOAD, hookId);
-    } else {
-      script =
-          new String(guideMarkableReaderTowardsEquality((Reader) arguments[0], PAYLOAD, hookId));
+      if (script.contains(PAYLOAD)) {
+        Jazzer.reportFindingFromHook(new FuzzerSecurityIssueCritical(
+            "Script Engine Injection: Insecure user input was used in script engine invocation.\n"
+            + "Depending on the script engine's capabilities this could lead to sandbox escape and remote code execution."));
+      }
+      Jazzer.guideTowardsContainment(script, PAYLOAD, hookId);
     }
+    return script;
+  }
 
-    if (script.equals(PAYLOAD)) {
-      Jazzer.reportFindingFromHook(new FuzzerSecurityIssueCritical("Possible script execution"));
-    }
+  private static String readAll(Reader reader) {
+    return new BufferedReader(reader).lines().collect(Collectors.joining());
   }
 }