From 7dcfa35e5bfbe6bfe86c90cc4a27cfc50ea1c9e7 Mon Sep 17 00:00:00 2001 From: Norbert Schneider Date: Wed, 31 May 2023 14:18:17 +0200 Subject: Reworked script engine detector Check for containment of payload in script content in all overloads of eval. --- .../main/java/com/example/CommonsTextFuzzer.java | 3 +- maven_install.json | 49 +++++- .../jazzer/sanitizers/BUILD.bazel | 2 +- .../jazzer/sanitizers/ScriptEngineInjection.java | 144 +++++++--------- .../java/com/example/ScriptEngineInjection.java | 188 ++++++++++++++------- 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. * *

* 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") 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 getScopes() { + return null; } } } -- cgit v1.2.3