Reworked script engine detector

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

5 files changed, 238 insertions, 148 deletions

diff --git a/examples/src/main/java/com/example/CommonsTextFuzzer.java b/examples/src/main/java/com/example/CommonsTextFuzzer.java
index ef93639c..32b309d5 100644
--- a/examples/src/main/java/com/example/CommonsTextFuzzer.java
+++ b/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,7 +15,6 @@
 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 {
diff --git a/maven_install.json b/maven_install.json
index efc0828c..588f75a0 100644
--- a/maven_install.json
+++ b/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",
diff --git a/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/BUILD.bazel b/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/BUILD.bazel
index 9d7a7a7d..c2521b80 100644
--- a/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/BUILD.bazel
+++ b/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/BUILD.bazel
@@ -52,8 +52,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 = [
diff --git a/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/ScriptEngineInjection.java b/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/ScriptEngineInjection.java
index e9500d6d..6f084bf9 100644
--- a/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/ScriptEngineInjection.java
+++ b/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,95 @@
 
 package com.code_intelligence.jazzer.sanitizers;
 
-import static java.util.Collections.unmodifiableSet;
-import static java.util.stream.Collectors.toSet;
-
 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.Reader;
+import java.io.StringReader;
 import java.lang.invoke.MethodHandle;
-import java.util.Arrays;
-import java.util.Set;
-import java.util.stream.Stream;
-import javax.script.ScriptEngineManager;
 
 /**
- * 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")
 public final class ScriptEngineInjection {
-  private static final String ENGINE = "js";
-  private static final String PAYLOAD = "1+1";
-
-  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;
-
-    if (!reader.markSupported()) {
-      throw new IllegalStateException("Reader does not support mark - not possible to fuzz");
-    }
-
-    reader.mark(size);
-
-    while (n < size) {
-      int count = reader.read(current, n, size - n);
-      if (count < 0)
-        break;
-      n += count;
-    }
-    reader.reset();
-
-    Jazzer.guideTowardsEquality(new String(current), target, id);
-
-    return current;
-  }
-
-  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngineManager",
-      targetMethod = "registerEngineName")
-  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());
-  }
-
-  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngineManager",
-      targetMethod = "getEngineByName",
-      targetMethodDescriptor = "(Ljava/lang/String;)Ljavax/script/ScriptEngine;")
-  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());
-  }
+  private static final String PAYLOAD = "\"jaz\"+\"zer\"";
 
+  /**
+   * String variants of eval can be intercepted by before hooks, as the script
+   * content can directly be checked for the presence of the payload.
+   */
   @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;")
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/lang/String;Ljavax/script/ScriptContext;)Ljava/lang/Object;")
+  @MethodHook(type = HookType.BEFORE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/lang/String;Ljavax/script/Bindings;)Ljava/lang/Object;")
   public static void
+  checkScriptEngineExecuteString(
+      MethodHandle method, Object thisObject, Object[] arguments, int hookId) {
+    checkScriptContent((String) arguments[0], hookId);
+  }
+
+  /**
+   * Reader variants of eval must be intercepted by replace hooks, as their
+   * contents are converted to strings, for the payload check, and back to readers
+   * for the actual method invocation.
+   */
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval", targetMethodDescriptor = "(Ljava/io/Reader;)Ljava/lang/Object;")
+  @MethodHook(type = HookType.REPLACE, targetClassName = "javax.script.ScriptEngine",
+      targetMethod = "eval",
+      targetMethodDescriptor = "(Ljava/io/Reader;Ljavax/script/ScriptContext;)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
   checkScriptEngineExecute(MethodHandle method, Object thisObject, Object[] arguments, int hookId)
       throws Throwable {
-    String script = null;
+    if (arguments[0] != null) {
+      String content = readAll((Reader) arguments[0]);
+      checkScriptContent(content, hookId);
+      arguments[0] = new StringReader(content);
+    }
+    return method.invokeWithArguments(thisObject, arguments);
+  }
 
-    if (arguments[0] instanceof String) {
-      script = (String) arguments[0];
-      Jazzer.guideTowardsEquality(script, PAYLOAD, hookId);
-    } else {
-      script =
-          new String(guideMarkableReaderTowardsEquality((Reader) arguments[0], PAYLOAD, hookId));
+  private static void checkScriptContent(String content, int hookId) {
+    if (content != null) {
+      if (content.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."));
+      } else {
+        Jazzer.guideTowardsContainment(content, PAYLOAD, hookId);
+      }
     }
+  }
 
-    if (script.equals(PAYLOAD)) {
-      Jazzer.reportFindingFromHook(new FuzzerSecurityIssueCritical("Possible script execution"));
+  private static String readAll(Reader reader) throws IOException {
+    StringBuilder content = new StringBuilder();
+    char[] buffer = new char[4096];
+    int numChars;
+    while ((numChars = reader.read(buffer)) >= 0) {
+      content.append(buffer, 0, numChars);
     }
+    return content.toString();
   }
 }
diff --git a/sanitizers/src/test/java/com/example/ScriptEngineInjection.java b/sanitizers/src/test/java/com/example/ScriptEngineInjection.java
index 0785348e..631b7ab8 100644
--- a/sanitizers/src/test/java/com/example/ScriptEngineInjection.java
+++ b/sanitizers/src/test/java/com/example/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.
@@ -15,95 +15,157 @@
 package com.example;
 
 import com.code_intelligence.jazzer.api.FuzzedDataProvider;
-import com.code_intelligence.jazzer.api.FuzzerSecurityIssueCritical;
 import java.io.Reader;
 import java.io.StringReader;
+import java.io.Writer;
+import java.util.List;
 import javax.script.Bindings;
 import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineFactory;
-import javax.script.ScriptEngineManager;
-import javax.script.ScriptException;
 
-class DummyScriptEngine implements ScriptEngine {
-  @Override
-  public Bindings createBindings() {
-    return null;
-  }
+public class ScriptEngineInjection {
+  private final static ScriptEngine engine = new DummyScriptEngine();
+  private final static ScriptContext context = new DummyScriptContext();
 
-  @Override
-  public Object eval(String script) throws ScriptException {
-    return null;
+  private static void insecureScriptEval(String input) throws Exception {
+    engine.eval(new StringReader(input), context);
   }
 
-  @Override
-  public Object eval(Reader reader) throws ScriptException {
-    return null;
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) throws Exception {
+    try {
+      insecureScriptEval(data.consumeRemainingAsAsciiString());
+    } catch (Exception ignored) {
+    }
   }
 
-  @Override
-  public Object eval(String script, ScriptContext context) throws ScriptException {
-    return null;
-  }
+  private static class DummyScriptEngine implements ScriptEngine {
+    @Override
+    public Bindings createBindings() {
+      return null;
+    }
 
-  @Override
-  public Object eval(Reader reader, ScriptContext context) throws ScriptException {
-    return null;
-  }
+    @Override
+    public Object eval(String script) {
+      return null;
+    }
 
-  @Override
-  public Object eval(String script, Bindings n) throws ScriptException {
-    return null;
-  }
+    @Override
+    public Object eval(Reader reader) {
+      return null;
+    }
 
-  @Override
-  public Object eval(Reader reader, Bindings n) throws ScriptException {
-    return null;
-  }
+    @Override
+    public Object eval(String script, ScriptContext context) {
+      return null;
+    }
 
-  @Override
-  public Object get(String key) {
-    return null;
-  }
+    @Override
+    public Object eval(Reader reader, ScriptContext context) {
+      return null;
+    }
 
-  @Override
-  public Bindings getBindings(int scope) {
-    return null;
-  }
+    @Override
+    public Object eval(String script, Bindings n) {
+      return null;
+    }
 
-  @Override
-  public ScriptContext getContext() {
-    return null;
-  }
+    @Override
+    public Object eval(Reader reader, Bindings n) {
+      return null;
+    }
 
-  @Override
-  public ScriptEngineFactory getFactory() {
-    return null;
-  }
+    @Override
+    public Object get(String key) {
+      return null;
+    }
 
-  @Override
-  public void put(String key, Object value) {}
+    @Override
+    public Bindings getBindings(int scope) {
+      return null;
+    }
 
-  @Override
-  public void setBindings(Bindings bindings, int scope) {}
+    @Override
+    public ScriptContext getContext() {
+      return null;
+    }
 
-  @Override
-  public void setContext(ScriptContext context) {}
+    @Override
+    public ScriptEngineFactory getFactory() {
+      return null;
+    }
 
-  public DummyScriptEngine() {}
-}
+    @Override
+    public void put(String key, Object value) {}
 
-public class ScriptEngineInjection {
-  static ScriptEngine engine = new DummyScriptEngine();
+    @Override
+    public void setBindings(Bindings bindings, int scope) {}
+
+    @Override
+    public void setContext(ScriptContext context) {}
 
-  static void insecureScriptEval(String input) throws Exception {
-    engine.eval(new StringReader(input));
+    public DummyScriptEngine() {}
   }
 
-  public static void fuzzerTestOneInput(FuzzedDataProvider data) throws Exception {
-    try {
-      insecureScriptEval(data.consumeRemainingAsAsciiString());
-    } catch (Exception ignored) {
+  private static class DummyScriptContext implements ScriptContext {
+    @Override
+    public void setBindings(Bindings bindings, int scope) {}
+
+    @Override
+    public Bindings getBindings(int scope) {
+      return null;
+    }
+
+    @Override
+    public void setAttribute(String name, Object value, int scope) {}
+
+    @Override
+    public Object getAttribute(String name, int scope) {
+      return null;
+    }
+
+    @Override
+    public Object removeAttribute(String name, int scope) {
+      return null;
+    }
+
+    @Override
+    public Object getAttribute(String name) {
+      return null;
+    }
+
+    @Override
+    public int getAttributesScope(String name) {
+      return 0;
+    }
+
+    @Override
+    public Writer getWriter() {
+      return null;
+    }
+
+    @Override
+    public Writer getErrorWriter() {
+      return null;
+    }
+
+    @Override
+    public void setWriter(Writer writer) {}
+
+    @Override
+    public void setErrorWriter(Writer writer) {}
+
+    @Override
+    public Reader getReader() {
+      return null;
+    }
+
+    @Override
+    public void setReader(Reader reader) {}
+
+    @Override
+    public List<Integer> getScopes() {
+      return null;
     }
   }
 }