From 2733872814c5e3c9e98a7f9590bbfc9cf7dd0800 Mon Sep 17 00:00:00 2001 From: Norbert Schneider Date: Wed, 31 May 2023 14:18:17 +0200 Subject: [PATCH] Reworked script engine detector Check for containment of payload in script content in all overloads of eval. --- examples/BUILD.bazel | 1 - .../java/com/example/CommonsTextFuzzer.java | 5 +- maven_install.json | 49 ++++- .../jazzer/sanitizers/BUILD.bazel | 3 +- .../sanitizers/ScriptEngineInjection.java | 172 +++++++++------- .../sanitizers/utils/ReflectionUtils.java | 29 ++- .../com/example/ScriptEngineInjection.java | 188 ++++++++++++------ 7 files changed, 301 insertions(+), 146 deletions(-) diff --git a/examples/BUILD.bazel b/examples/BUILD.bazel index 1a7da538b..fa88ac413 100644 --- a/examples/BUILD.bazel +++ b/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"], diff --git a/examples/src/main/java/com/example/CommonsTextFuzzer.java b/examples/src/main/java/com/example/CommonsTextFuzzer.java index ef93639c1..ab0e69a5f 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,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) { } diff --git a/maven_install.json b/maven_install.json index efc0828cf..588f75a06 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 9d7a7a7d3..70b7c8f19 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 @@ -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 = [ 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 e9500d6d0..a555ae927 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,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. * *

* 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 JSR-223 + * execution of arbitrary OS commands. Several other scripting engines can be + * embedded to the JVM (they must follow the + * JSR-223 * specification). - *

**/ +@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 eval; + private static Optional evalWithScriptContext; + private static Optional 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()); } } diff --git a/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/utils/ReflectionUtils.java b/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/utils/ReflectionUtils.java index fd6ac72fa..d34942a68 100644 --- a/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/utils/ReflectionUtils.java +++ b/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/utils/ReflectionUtils.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,6 +15,9 @@ package com.code_intelligence.jazzer.sanitizers.utils; import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Optional; import sun.misc.Unsafe; public final class ReflectionUtils { @@ -54,6 +57,30 @@ public static Field field(Class clazz, String name, Class type) { } } + public static Optional methodFromObject(Object obj, String name, String... params) { + try { + Class thisClass = obj.getClass(); + Class[] paramClasses = Arrays.stream(params) + .map(param -> { + try { + // Use class loader of thisClass, as that is most likely to + // have access to the parameter classes as well. E.g. + // bootstrap class loader can not access javax classes + // anymore. + return thisClass.getClassLoader().loadClass(param); + } catch (ClassNotFoundException ignored) { + return null; + } + }) + .toArray(Class[] ::new); + return Optional.of(thisClass.getMethod(name, paramClasses)); + } catch (Throwable e) { + if (JAZZER_REFLECTION_DEBUG) + e.printStackTrace(); + return Optional.empty(); + } + } + public static long offset(Unsafe unsafe, Field field) { if (unsafe == null || field == null) return INVALID_OFFSET; diff --git a/sanitizers/src/test/java/com/example/ScriptEngineInjection.java b/sanitizers/src/test/java/com/example/ScriptEngineInjection.java index 0785348e9..631b7ab8d 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 getScopes() { + return null; } } }