From 73e940de74bb326664db451b97fa58e67e683400 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 2 Feb 2021 17:37:14 +0100 Subject: [PATCH 01/10] Added query for Jakarta EL injections - Added JakartaExpressionInjection.ql - Added a qhelp file with examples --- .../Security/CWE/CWE-094/InjectionLib.qll | 14 +++ .../CWE-094/JakartaExpressionInjection.qhelp | 65 ++++++++++++ .../CWE/CWE-094/JakartaExpressionInjection.ql | 20 ++++ .../CWE-094/JakartaExpressionInjectionLib.qll | 98 +++++++++++++++++++ .../Security/CWE/CWE-094/JexlInjectionLib.qll | 13 +-- .../SaferExpressionEvaluationWithJuel.java | 10 ++ .../UnsafeExpressionEvaluationWithJuel.java | 5 + 7 files changed, 213 insertions(+), 12 deletions(-) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll new file mode 100644 index 000000000000..adab79d6f5c3 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll @@ -0,0 +1,14 @@ +import java +import semmle.code.java.dataflow.FlowSources + +/** + * Holds if `fromNode` to `toNode` is a dataflow step that returns data from + * a bean by calling one of its getters. + */ +predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node toNode) { + exists(MethodAccess ma, Method m | ma.getMethod() = m | + m instanceof GetterMethod and + ma.getQualifier() = fromNode.asExpr() and + ma = toNode.asExpr() + ) +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp new file mode 100644 index 000000000000..6e6b5f633947 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp @@ -0,0 +1,65 @@ + + + + +

+Jakarta Expression Language (EL) is an expression language for Java applications. +There are a single language specification and multiple implementations +such as Glassfish, Juel, Apache Commons EL, etc. +The language allows invocation of methods available in the JVM. +If an expression is built using attacker-controlled data, +and then evaluated, then it may allow the attacker to run arbitrary code. +

+ + + +

+It is generally recommended to avoid using untrusted data in an EL expression. +Before using untrusted data to build an EL expressoin, the data should be validated +to ensure it is not evaluated as expression language. If the EL implementaion offers +configuring a sandbox for EL expression, they should be run in a restircitive sandbox +that allows accessing only explicitly allowed classes. If the EL implementation +does not allow sandboxing, consider using other expressiong language implementations +with sandboxing capabilities such as Apache Commons JEXL or the Spring Expression Language. +

+ + + +

+The following example shows how untrusted data is used to build and run an expression +using the JUEL interpreter: +

+ + +

+JUEL does not allow to run expression in a sandbox. To prevent running arbitrary code, +incoming data has to be checked before including to an expression. The next example +uses a Regex pattern to check whether a user tries to run an allowed exression or not: +

+ + + + + +
  • + Eclipse Foundation: + Jakarta Expression Language. +
    • +
  • + Jakarta EE documentation: + Jakarta Expression Language API +
    • +
  • + OWASP: + Expression Language Injection. +
    • +
  • + JUEL: + Home page +
    • +
  • + Apache Foundation: + Apache Commons EL +
    • +     +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql new file mode 100644 index 000000000000..b94c98201dea --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql @@ -0,0 +1,20 @@ +/** + * @name Java EE Expression Language injection + * @description Evaluation of a user-controlled Jave EE expression + * may lead to arbitrary code execution. + * @kind path-problem + * @problem.severity error + * @precision high + * @id java/javaee-expression-injection + * @tags security + * external/cwe/cwe-094 + */ + +import java +import JavaEEExpressionInjectionLib +import DataFlow::PathGraph + +from DataFlow::PathNode source, DataFlow::PathNode sink, JavaEEExpressionInjectionConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Java EE Expression Language injection from $@.", + source.getNode(), "this user input" diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll new file mode 100644 index 000000000000..d43b31c98881 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -0,0 +1,98 @@ +import java +import InjectionLib +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking + +/** + * A taint-tracking configuration for unsafe user input + * that is used to construct and evaluate a Java EE expression. + */ +class JavaEEExpressionInjectionConfig extends TaintTracking::Configuration { + JavaEEExpressionInjectionConfig() { this = "JavaEEExpressionInjectionConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink } + + override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + any(TaintPropagatingCall c).taintFlow(fromNode, toNode) or + returnsDataFromBean(fromNode, toNode) + } +} + +/** + * A sink for Expresssion Language injection vulnerabilities, + * i.e. method calls that run evaluation of a Java EE expression. + */ +private class ExpressionEvaluationSink extends DataFlow::ExprNode { + ExpressionEvaluationSink() { + exists(MethodAccess ma, Method m, Expr taintFrom | + ma.getMethod() = m and taintFrom = this.asExpr() + | + m.getDeclaringType() instanceof ValueExpression and + m.hasName(["getValue", "setValue"]) and + ma.getQualifier() = taintFrom + or + m.getDeclaringType() instanceof MethodExpression and + m.hasName("invoke") and + ma.getQualifier() = taintFrom + or + m.getDeclaringType() instanceof LambdaExpression and + m.hasName("invoke") and + ma.getQualifier() = taintFrom + or + m.getDeclaringType() instanceof ELProcessor and + m.hasName(["eval", "getValue", "setValue"]) and + ma.getArgument(0) = taintFrom + ) + } +} + +/** + * Defines method calls that propagate tainted expressions. + */ +private class TaintPropagatingCall extends Call { + Expr taintFromExpr; + + TaintPropagatingCall() { + taintFromExpr = this.getArgument(1) and + exists(Method m | this.(MethodAccess).getMethod() = m | + m.getDeclaringType() instanceof ExpressionFactory and + m.hasName(["createValueExpression", "createMethodExpression"]) and + taintFromExpr.getType() instanceof TypeString + ) + or + exists(Constructor c | this.(ConstructorCall).getConstructor() = c | + c.getDeclaringType() instanceof LambdaExpression and + taintFromExpr.getType() instanceof ValueExpression + ) + } + + /** + * Holds if `fromNode` to `toNode` is a dataflow step that propagates + * tainted data. + */ + predicate taintFlow(DataFlow::Node fromNode, DataFlow::Node toNode) { + fromNode.asExpr() = taintFromExpr and toNode.asExpr() = this + } +} + +private class ELProcessor extends RefType { + ELProcessor() { hasQualifiedName("javax.el", "ELProcessor") } +} + +private class ExpressionFactory extends RefType { + ExpressionFactory() { hasQualifiedName("javax.el", "ExpressionFactory") } +} + +private class ValueExpression extends RefType { + ValueExpression() { hasQualifiedName("javax.el", "ValueExpression") } +} + +private class MethodExpression extends RefType { + MethodExpression() { hasQualifiedName("javax.el", "MethodExpression") } +} + +private class LambdaExpression extends RefType { + LambdaExpression() { hasQualifiedName("javax.el", "LambdaExpression") } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll index 561d7e46ae90..51084a9862ce 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll @@ -1,4 +1,5 @@ import java +import InjectionLib import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking @@ -152,18 +153,6 @@ private predicate createsJexlEngine(DataFlow::Node fromNode, DataFlow::Node toNo ) } -/** - * Holds if `fromNode` to `toNode` is a dataflow step that returns data from - * a bean by calling one of its getters. - */ -private predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node toNode) { - exists(MethodAccess ma, Method m | ma.getMethod() = m | - m instanceof GetterMethod and - ma.getQualifier() = fromNode.asExpr() and - ma = toNode.asExpr() - ) -} - /** * A methods in the `JexlEngine` class that gets or sets a property with a JEXL expression. */ diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java b/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java new file mode 100644 index 000000000000..54fb9a0ed36c --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java @@ -0,0 +1,10 @@ +String input = getRemoteUserInput(); +String pattern = "(inside|outside)\\.(temperature|humidity)"; +if (!input.matches(pattern)) { + throw new IllegalArgumentException("Unexpected exression"); +} +String expression = "${" + input + "}"; +ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); +ValueExpression e = factory.createValueExpression(context, expression, Object.class); +SimpleContext context = getContext(); +Object result = e.getValue(context); \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java b/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java new file mode 100644 index 000000000000..27afa0fcb497 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeExpressionEvaluationWithJuel.java @@ -0,0 +1,5 @@ +String expression = "${" + getRemoteUserInput() + "}"; +ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); +ValueExpression e = factory.createValueExpression(context, expression, Object.class); +SimpleContext context = getContext(); +Object result = e.getValue(context); \ No newline at end of file From adb1ed380ac4736860eac8ea6cfadb8f38497db5 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sun, 21 Mar 2021 20:36:47 +0300 Subject: [PATCH 02/10] Added tests for Jakarta expression injection --- .../CWE/CWE-094/JakartaExpressionInjection.ql | 10 +-- .../CWE-094/JakartaExpressionInjectionLib.qll | 8 +- .../JakartaExpressionInjection.expected | 59 +++++++++++++ .../CWE-094/JakartaExpressionInjection.java | 87 +++++++++++++++++++ .../CWE-094/JakartaExpressionInjection.qlref | 1 + .../query-tests/security/CWE-094/options | 3 +- .../stubs/java-ee-el/javax/el/ELContext.java | 3 + .../stubs/java-ee-el/javax/el/ELManager.java | 5 ++ .../java-ee-el/javax/el/ELProcessor.java | 7 ++ .../javax/el/ExpressionFactory.java | 17 ++++ .../java-ee-el/javax/el/LambdaExpression.java | 8 ++ .../java-ee-el/javax/el/MethodExpression.java | 5 ++ .../javax/el/StandardELContext.java | 5 ++ .../java-ee-el/javax/el/ValueExpression.java | 6 ++ .../de/odysseus/el/ExpressionFactoryImpl.java | 5 ++ .../de/odysseus/el/util/SimpleContext.java | 5 ++ 16 files changed, 223 insertions(+), 11 deletions(-) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.qlref create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/ELContext.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/ELManager.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/ExpressionFactory.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/LambdaExpression.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/MethodExpression.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/StandardELContext.java create mode 100644 java/ql/test/stubs/java-ee-el/javax/el/ValueExpression.java create mode 100644 java/ql/test/stubs/juel-2.2/de/odysseus/el/ExpressionFactoryImpl.java create mode 100644 java/ql/test/stubs/juel-2.2/de/odysseus/el/util/SimpleContext.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql index b94c98201dea..8190ec3d61f1 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql @@ -1,6 +1,6 @@ /** - * @name Java EE Expression Language injection - * @description Evaluation of a user-controlled Jave EE expression + * @name Jakarta Expression Language injection + * @description Evaluation of a user-controlled expression * may lead to arbitrary code execution. * @kind path-problem * @problem.severity error @@ -11,10 +11,10 @@ */ import java -import JavaEEExpressionInjectionLib +import JakartaExpressionInjectionLib import DataFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, JavaEEExpressionInjectionConfig conf +from DataFlow::PathNode source, DataFlow::PathNode sink, JakartaExpressionInjectionConfig conf where conf.hasFlowPath(source, sink) -select sink.getNode(), source, sink, "Java EE Expression Language injection from $@.", +select sink.getNode(), source, sink, "Jakarta Expression Language injection from $@.", source.getNode(), "this user input" diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll index d43b31c98881..bc22f7c32572 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -5,10 +5,10 @@ import semmle.code.java.dataflow.TaintTracking /** * A taint-tracking configuration for unsafe user input - * that is used to construct and evaluate a Java EE expression. + * that is used to construct and evaluate an expression. */ -class JavaEEExpressionInjectionConfig extends TaintTracking::Configuration { - JavaEEExpressionInjectionConfig() { this = "JavaEEExpressionInjectionConfig" } +class JakartaExpressionInjectionConfig extends TaintTracking::Configuration { + JakartaExpressionInjectionConfig() { this = "JakartaExpressionInjectionConfig" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } @@ -22,7 +22,7 @@ class JavaEEExpressionInjectionConfig extends TaintTracking::Configuration { /** * A sink for Expresssion Language injection vulnerabilities, - * i.e. method calls that run evaluation of a Java EE expression. + * i.e. method calls that run evaluation of an expression. */ private class ExpressionEvaluationSink extends DataFlow::ExprNode { ExpressionEvaluationSink() { diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected new file mode 100644 index 000000000000..111cc81541c2 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected @@ -0,0 +1,59 @@ +edges +| JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:24:31:24:40 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:30:24:30:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:37:24:37:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:44:24:44:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:54:24:54:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:61:24:61:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:70:24:70:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:79:24:79:33 | expression : String | +| JakartaExpressionInjection.java:30:24:30:33 | expression : String | JakartaExpressionInjection.java:32:28:32:37 | expression | +| JakartaExpressionInjection.java:37:24:37:33 | expression : String | JakartaExpressionInjection.java:39:32:39:41 | expression | +| JakartaExpressionInjection.java:44:24:44:33 | expression : String | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | +| JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | +| JakartaExpressionInjection.java:54:24:54:33 | expression : String | JakartaExpressionInjection.java:56:32:56:41 | expression | +| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | +| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:65:13:65:13 | e | +| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | +| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | +| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:65:13:65:13 | e | +| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | +| JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | +| JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | +| JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:74:13:74:13 | e | +| JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | +| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | +| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:74:13:74:13 | e | +| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | +| JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | +| JakartaExpressionInjection.java:79:24:79:33 | expression : String | JakartaExpressionInjection.java:83:13:83:13 | e | +nodes +| JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:30:24:30:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:32:28:32:37 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:37:24:37:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:39:32:39:41 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:44:24:44:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | semmle.label | new LambdaExpression(...) : LambdaExpression | +| JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | semmle.label | lambdaExpression | +| JakartaExpressionInjection.java:54:24:54:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:56:32:56:41 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:61:24:61:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | semmle.label | createValueExpression(...) : ValueExpression | +| JakartaExpressionInjection.java:65:13:65:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | semmle.label | e : ValueExpression | +| JakartaExpressionInjection.java:70:24:70:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | semmle.label | createValueExpression(...) : ValueExpression | +| JakartaExpressionInjection.java:74:13:74:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | semmle.label | e : ValueExpression | +| JakartaExpressionInjection.java:79:24:79:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:83:13:83:13 | e | semmle.label | e | +#select +| JakartaExpressionInjection.java:32:28:32:37 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:32:28:32:37 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:39:32:39:41 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:39:32:39:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:56:32:56:41 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:56:32:56:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:65:13:65:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:65:13:65:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:74:13:74:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:74:13:74:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:83:13:83:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:83:13:83:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java new file mode 100644 index 000000000000..a9fb2d45d6f4 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java @@ -0,0 +1,87 @@ +import java.io.IOException; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.ArrayList; +import java.util.function.Consumer; + +import javax.el.ELContext; +import javax.el.ELManager; +import javax.el.ELProcessor; +import javax.el.ExpressionFactory; +import javax.el.LambdaExpression; +import javax.el.MethodExpression; +import javax.el.StandardELContext; +import javax.el.ValueExpression; + +public class JakartaExpressionInjection { + + private static void testWithSocket(Consumer action) throws IOException { + try (ServerSocket serverSocket = new ServerSocket(0)) { + try (Socket socket = serverSocket.accept()) { + byte[] bytes = new byte[1024]; + int n = socket.getInputStream().read(bytes); + String expression = new String(bytes, 0, n); + action.accept(expression); + } + } + } + + private static void testWithELProcessorEval() throws IOException { + testWithSocket(expression -> { + ELProcessor processor = new ELProcessor(); + processor.eval(expression); + }); + } + + private static void testWithELProcessorGetValue() throws IOException { + testWithSocket(expression -> { + ELProcessor processor = new ELProcessor(); + processor.getValue(expression, Object.class); + }); + } + + private static void testWithLambdaExpressionInvoke() throws IOException { + testWithSocket(expression -> { + ExpressionFactory factory = ELManager.getExpressionFactory(); + StandardELContext context = new StandardELContext(factory); + ValueExpression valueExpression = factory.createValueExpression(context, expression, Object.class); + LambdaExpression lambdaExpression = new LambdaExpression(new ArrayList<>(), valueExpression); + lambdaExpression.invoke(context, new Object[0]); + }); + } + + private static void testWithELProcessorSetValue() throws IOException { + testWithSocket(expression -> { + ELProcessor processor = new ELProcessor(); + processor.setValue(expression, new Object()); + }); + } + + private static void testWithJuelValueExpressionGetValue() throws IOException { + testWithSocket(expression -> { + ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); + ELContext context = new de.odysseus.el.util.SimpleContext(); + ValueExpression e = factory.createValueExpression(context, expression, Object.class); + e.getValue(context); + }); + } + + private static void testWithJuelValueExpressionSetValue() throws IOException { + testWithSocket(expression -> { + ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); + ELContext context = new de.odysseus.el.util.SimpleContext(); + ValueExpression e = factory.createValueExpression(context, expression, Object.class); + e.setValue(context, new Object()); + }); + } + + private static void testWithJuelMethodExpressionInvoke() throws IOException { + testWithSocket(expression -> { + ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); + ELContext context = new de.odysseus.el.util.SimpleContext(); + MethodExpression e = factory.createMethodExpression(context, expression, Object.class, new Class[0]); + e.invoke(context, new Object[0]); + }); + } + +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.qlref b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.qlref new file mode 100644 index 000000000000..3154ee5ccad4 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-094/JakartaExpressionInjection.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/options b/java/ql/test/experimental/query-tests/security/CWE-094/options index a8e30ce30b40..ec3354ea41ce 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/options +++ b/java/ql/test/experimental/query-tests/security/CWE-094/options @@ -1,2 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../../stubs/apache-commons-jexl-3.1:${testdir}/../../../../stubs/scriptengine - +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../../stubs/apache-commons-jexl-3.1:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2 \ No newline at end of file diff --git a/java/ql/test/stubs/java-ee-el/javax/el/ELContext.java b/java/ql/test/stubs/java-ee-el/javax/el/ELContext.java new file mode 100644 index 000000000000..ce3840c69c8d --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/ELContext.java @@ -0,0 +1,3 @@ +package javax.el; + +public class ELContext {} diff --git a/java/ql/test/stubs/java-ee-el/javax/el/ELManager.java b/java/ql/test/stubs/java-ee-el/javax/el/ELManager.java new file mode 100644 index 000000000000..7d24f739a3f8 --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/ELManager.java @@ -0,0 +1,5 @@ +package javax.el; + +public class ELManager { + public static ExpressionFactory getExpressionFactory() { return null; } +} \ No newline at end of file diff --git a/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java b/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java new file mode 100644 index 000000000000..3c523e685c5e --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java @@ -0,0 +1,7 @@ +package javax.el; + +public class ELProcessor { + public Object eval(String expression) { return null; } + public Object getValue(String expression, Class expectedType) { return null; } + public void setValue(String expression, Object value) {} +} diff --git a/java/ql/test/stubs/java-ee-el/javax/el/ExpressionFactory.java b/java/ql/test/stubs/java-ee-el/javax/el/ExpressionFactory.java new file mode 100644 index 000000000000..31ff79169acb --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/ExpressionFactory.java @@ -0,0 +1,17 @@ +package javax.el; + +public class ExpressionFactory { + public MethodExpression createMethodExpression(ELContext context, String expression, Class expectedReturnType, + Class[] expectedParamTypes) { + + return null; + } + + public ValueExpression createValueExpression(ELContext context, String expression, Class expectedType) { + return null; + } + + public ValueExpression createValueExpression(Object instance, Class expectedType) { + return null; + } +} diff --git a/java/ql/test/stubs/java-ee-el/javax/el/LambdaExpression.java b/java/ql/test/stubs/java-ee-el/javax/el/LambdaExpression.java new file mode 100644 index 000000000000..4be01e9d2e4b --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/LambdaExpression.java @@ -0,0 +1,8 @@ +package javax.el; + +import java.util.List; + +public class LambdaExpression { + public LambdaExpression(List formalParameters, ValueExpression expression) {} + public Object invoke(Object... args) { return null; } +} diff --git a/java/ql/test/stubs/java-ee-el/javax/el/MethodExpression.java b/java/ql/test/stubs/java-ee-el/javax/el/MethodExpression.java new file mode 100644 index 000000000000..ac50ece80e36 --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/MethodExpression.java @@ -0,0 +1,5 @@ +package javax.el; + +public class MethodExpression { + public Object invoke(ELContext context, Object[] params) { return null; } +} diff --git a/java/ql/test/stubs/java-ee-el/javax/el/StandardELContext.java b/java/ql/test/stubs/java-ee-el/javax/el/StandardELContext.java new file mode 100644 index 000000000000..22e3f2a9fc13 --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/StandardELContext.java @@ -0,0 +1,5 @@ +package javax.el; + +public class StandardELContext extends ELContext { + public StandardELContext(ExpressionFactory factory) {} +} diff --git a/java/ql/test/stubs/java-ee-el/javax/el/ValueExpression.java b/java/ql/test/stubs/java-ee-el/javax/el/ValueExpression.java new file mode 100644 index 000000000000..9a231215640a --- /dev/null +++ b/java/ql/test/stubs/java-ee-el/javax/el/ValueExpression.java @@ -0,0 +1,6 @@ +package javax.el; + +public class ValueExpression { + public Object getValue(ELContext context) { return null; } + public void setValue(ELContext context, Object value) {} +} diff --git a/java/ql/test/stubs/juel-2.2/de/odysseus/el/ExpressionFactoryImpl.java b/java/ql/test/stubs/juel-2.2/de/odysseus/el/ExpressionFactoryImpl.java new file mode 100644 index 000000000000..a555cf88dee0 --- /dev/null +++ b/java/ql/test/stubs/juel-2.2/de/odysseus/el/ExpressionFactoryImpl.java @@ -0,0 +1,5 @@ +package de.odysseus.el; + +import javax.el.ExpressionFactory; + +public class ExpressionFactoryImpl extends ExpressionFactory {} diff --git a/java/ql/test/stubs/juel-2.2/de/odysseus/el/util/SimpleContext.java b/java/ql/test/stubs/juel-2.2/de/odysseus/el/util/SimpleContext.java new file mode 100644 index 000000000000..aa4f449e5fa5 --- /dev/null +++ b/java/ql/test/stubs/juel-2.2/de/odysseus/el/util/SimpleContext.java @@ -0,0 +1,5 @@ +package de.odysseus.el.util; + +import javax.el.ELContext; + +public class SimpleContext extends ELContext {} From 6c246994035a35925d870f97abc58c04162d53f9 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sun, 21 Mar 2021 21:16:00 +0300 Subject: [PATCH 03/10] Cover both javax.el and jakarta.el packages --- .../CWE-094/JakartaExpressionInjectionLib.qll | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll index bc22f7c32572..22f421b8bf8c 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -77,22 +77,26 @@ private class TaintPropagatingCall extends Call { } } -private class ELProcessor extends RefType { - ELProcessor() { hasQualifiedName("javax.el", "ELProcessor") } +private class JakartaType extends RefType { + JakartaType() { getPackage().hasName(["javax.el", "jakarta.el"]) } } -private class ExpressionFactory extends RefType { - ExpressionFactory() { hasQualifiedName("javax.el", "ExpressionFactory") } +private class ELProcessor extends JakartaType { + ELProcessor() { hasName("ELProcessor") } } -private class ValueExpression extends RefType { - ValueExpression() { hasQualifiedName("javax.el", "ValueExpression") } +private class ExpressionFactory extends JakartaType { + ExpressionFactory() { hasName("ExpressionFactory") } } -private class MethodExpression extends RefType { - MethodExpression() { hasQualifiedName("javax.el", "MethodExpression") } +private class ValueExpression extends JakartaType { + ValueExpression() { hasName("ValueExpression") } +} + +private class MethodExpression extends JakartaType { + MethodExpression() { hasName("MethodExpression") } } private class LambdaExpression extends RefType { - LambdaExpression() { hasQualifiedName("javax.el", "LambdaExpression") } + LambdaExpression() { hasName("LambdaExpression") } } From 80ac2aff268cd11501e8aaea05842c63bbc7afb4 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 7 Apr 2021 20:55:03 +0300 Subject: [PATCH 04/10] Fixed typos Co-authored-by: Marcono1234 --- .../CWE-094/JakartaExpressionInjection.qhelp | 18 +++++++++--------- .../SaferExpressionEvaluationWithJuel.java | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp index 6e6b5f633947..8a9e4e7276f0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp @@ -4,22 +4,22 @@

    Jakarta Expression Language (EL) is an expression language for Java applications. -There are a single language specification and multiple implementations +There is a single language specification and multiple implementations such as Glassfish, Juel, Apache Commons EL, etc. The language allows invocation of methods available in the JVM. If an expression is built using attacker-controlled data, -and then evaluated, then it may allow the attacker to run arbitrary code. +and then evaluated, it may allow the attacker to run arbitrary code.

    It is generally recommended to avoid using untrusted data in an EL expression. -Before using untrusted data to build an EL expressoin, the data should be validated -to ensure it is not evaluated as expression language. If the EL implementaion offers -configuring a sandbox for EL expression, they should be run in a restircitive sandbox +Before using untrusted data to build an EL expression, the data should be validated +to ensure it is not evaluated as expression language. If the EL implementation offers +configuring a sandbox for EL expressions, they should be run in a restrictive sandbox that allows accessing only explicitly allowed classes. If the EL implementation -does not allow sandboxing, consider using other expressiong language implementations +does not support sandboxing, consider using other expression language implementations with sandboxing capabilities such as Apache Commons JEXL or the Spring Expression Language.

    @@ -32,9 +32,9 @@ using the JUEL interpreter:

    -JUEL does not allow to run expression in a sandbox. To prevent running arbitrary code, -incoming data has to be checked before including to an expression. The next example -uses a Regex pattern to check whether a user tries to run an allowed exression or not: +JUEL does not support to run expressions in a sandbox. To prevent running arbitrary code, +incoming data has to be checked before including it in an expression. The next example +uses a Regex pattern to check whether a user tries to run an allowed expression or not:

    diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java b/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java index 54fb9a0ed36c..3dfaaead68a5 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java +++ b/java/ql/src/experimental/Security/CWE/CWE-094/SaferExpressionEvaluationWithJuel.java @@ -1,10 +1,10 @@ String input = getRemoteUserInput(); String pattern = "(inside|outside)\\.(temperature|humidity)"; if (!input.matches(pattern)) { - throw new IllegalArgumentException("Unexpected exression"); + throw new IllegalArgumentException("Unexpected expression"); } String expression = "${" + input + "}"; ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); ValueExpression e = factory.createValueExpression(context, expression, Object.class); SimpleContext context = getContext(); -Object result = e.getValue(context); \ No newline at end of file +Object result = e.getValue(context); From 3d8e173c5770a32804cbffb798ce63cfaddb795d Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 7 Apr 2021 20:59:07 +0300 Subject: [PATCH 05/10] Removed a reference to Apache Commons EL --- .../Security/CWE/CWE-094/JakartaExpressionInjection.qhelp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp index 8a9e4e7276f0..a6d66c1d1d16 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp @@ -57,9 +57,5 @@ uses a Regex pattern to check whether a user tries to run an allowed expression JUEL: Home page -
  • - Apache Foundation: - Apache Commons EL -
    • From c13ee0859acb4222aae8d4071d039494edfc782a Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 7 Apr 2021 21:02:21 +0300 Subject: [PATCH 06/10] LambdaExpression should extend JakartaType --- .../Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll index 22f421b8bf8c..5b652d3d8e65 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -97,6 +97,6 @@ private class MethodExpression extends JakartaType { MethodExpression() { hasName("MethodExpression") } } -private class LambdaExpression extends RefType { +private class LambdaExpression extends JakartaType { LambdaExpression() { hasName("LambdaExpression") } } From a764a79090e17ce22b7e1a3d8927dc80f0effd1b Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 7 Apr 2021 21:12:21 +0300 Subject: [PATCH 07/10] Always bind arguments in TaintPropagatingCall --- .../CWE-094/JakartaExpressionInjectionLib.qll | 20 ++++++++++--------- .../JakartaExpressionInjection.expected | 18 ----------------- 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll index 5b652d3d8e65..e3a3994c8818 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -56,15 +56,17 @@ private class TaintPropagatingCall extends Call { TaintPropagatingCall() { taintFromExpr = this.getArgument(1) and - exists(Method m | this.(MethodAccess).getMethod() = m | - m.getDeclaringType() instanceof ExpressionFactory and - m.hasName(["createValueExpression", "createMethodExpression"]) and - taintFromExpr.getType() instanceof TypeString - ) - or - exists(Constructor c | this.(ConstructorCall).getConstructor() = c | - c.getDeclaringType() instanceof LambdaExpression and - taintFromExpr.getType() instanceof ValueExpression + ( + exists(Method m | this.(MethodAccess).getMethod() = m | + m.getDeclaringType() instanceof ExpressionFactory and + m.hasName(["createValueExpression", "createMethodExpression"]) and + taintFromExpr.getType() instanceof TypeString + ) + or + exists(Constructor c | this.(ConstructorCall).getConstructor() = c | + c.getDeclaringType() instanceof LambdaExpression and + taintFromExpr.getType() instanceof ValueExpression + ) ) } diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected index 111cc81541c2..e0d699b14e3a 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected @@ -10,22 +10,9 @@ edges | JakartaExpressionInjection.java:30:24:30:33 | expression : String | JakartaExpressionInjection.java:32:28:32:37 | expression | | JakartaExpressionInjection.java:37:24:37:33 | expression : String | JakartaExpressionInjection.java:39:32:39:41 | expression | | JakartaExpressionInjection.java:44:24:44:33 | expression : String | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | -| JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | | JakartaExpressionInjection.java:54:24:54:33 | expression : String | JakartaExpressionInjection.java:56:32:56:41 | expression | -| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | | JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:65:13:65:13 | e | -| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | -| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | -| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:65:13:65:13 | e | -| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | -| JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | -| JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | | JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:74:13:74:13 | e | -| JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | -| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | -| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:74:13:74:13 | e | -| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | -| JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | | JakartaExpressionInjection.java:79:24:79:33 | expression : String | JakartaExpressionInjection.java:83:13:83:13 | e | nodes | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | @@ -35,18 +22,13 @@ nodes | JakartaExpressionInjection.java:37:24:37:33 | expression : String | semmle.label | expression : String | | JakartaExpressionInjection.java:39:32:39:41 | expression | semmle.label | expression | | JakartaExpressionInjection.java:44:24:44:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:48:49:48:104 | new LambdaExpression(...) : LambdaExpression | semmle.label | new LambdaExpression(...) : LambdaExpression | | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | semmle.label | lambdaExpression | | JakartaExpressionInjection.java:54:24:54:33 | expression : String | semmle.label | expression : String | | JakartaExpressionInjection.java:56:32:56:41 | expression | semmle.label | expression | | JakartaExpressionInjection.java:61:24:61:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:64:33:64:96 | createValueExpression(...) : ValueExpression | semmle.label | createValueExpression(...) : ValueExpression | | JakartaExpressionInjection.java:65:13:65:13 | e | semmle.label | e | -| JakartaExpressionInjection.java:65:13:65:13 | e : ValueExpression | semmle.label | e : ValueExpression | | JakartaExpressionInjection.java:70:24:70:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:73:33:73:96 | createValueExpression(...) : ValueExpression | semmle.label | createValueExpression(...) : ValueExpression | | JakartaExpressionInjection.java:74:13:74:13 | e | semmle.label | e | -| JakartaExpressionInjection.java:74:13:74:13 | e : ValueExpression | semmle.label | e : ValueExpression | | JakartaExpressionInjection.java:79:24:79:33 | expression : String | semmle.label | expression : String | | JakartaExpressionInjection.java:83:13:83:13 | e | semmle.label | e | #select From b39a3ab12ccd57ec76fc22b58d2fde78aadf38c7 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 8 Apr 2021 20:32:10 +0300 Subject: [PATCH 08/10] Added setVariable() sink --- .../CWE-094/JakartaExpressionInjectionLib.qll | 4 +++ .../JakartaExpressionInjection.expected | 31 +++++++++++-------- .../CWE-094/JakartaExpressionInjection.java | 7 +++++ .../java-ee-el/javax/el/ELProcessor.java | 1 + 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll index e3a3994c8818..b1c5d1e8ae20 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -44,6 +44,10 @@ private class ExpressionEvaluationSink extends DataFlow::ExprNode { m.getDeclaringType() instanceof ELProcessor and m.hasName(["eval", "getValue", "setValue"]) and ma.getArgument(0) = taintFrom + or + m.getDeclaringType() instanceof ELProcessor and + m.hasName("setVariable") and + ma.getArgument(1) = taintFrom ) } } diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected index e0d699b14e3a..ef002aefdf5c 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected @@ -5,15 +5,17 @@ edges | JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:44:24:44:33 | expression : String | | JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:54:24:54:33 | expression : String | | JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:61:24:61:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:70:24:70:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:79:24:79:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:68:24:68:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:77:24:77:33 | expression : String | +| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:86:24:86:33 | expression : String | | JakartaExpressionInjection.java:30:24:30:33 | expression : String | JakartaExpressionInjection.java:32:28:32:37 | expression | | JakartaExpressionInjection.java:37:24:37:33 | expression : String | JakartaExpressionInjection.java:39:32:39:41 | expression | | JakartaExpressionInjection.java:44:24:44:33 | expression : String | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | | JakartaExpressionInjection.java:54:24:54:33 | expression : String | JakartaExpressionInjection.java:56:32:56:41 | expression | -| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:65:13:65:13 | e | -| JakartaExpressionInjection.java:70:24:70:33 | expression : String | JakartaExpressionInjection.java:74:13:74:13 | e | -| JakartaExpressionInjection.java:79:24:79:33 | expression : String | JakartaExpressionInjection.java:83:13:83:13 | e | +| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:63:43:63:52 | expression | +| JakartaExpressionInjection.java:68:24:68:33 | expression : String | JakartaExpressionInjection.java:72:13:72:13 | e | +| JakartaExpressionInjection.java:77:24:77:33 | expression : String | JakartaExpressionInjection.java:81:13:81:13 | e | +| JakartaExpressionInjection.java:86:24:86:33 | expression : String | JakartaExpressionInjection.java:90:13:90:13 | e | nodes | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | | JakartaExpressionInjection.java:24:31:24:40 | expression : String | semmle.label | expression : String | @@ -26,16 +28,19 @@ nodes | JakartaExpressionInjection.java:54:24:54:33 | expression : String | semmle.label | expression : String | | JakartaExpressionInjection.java:56:32:56:41 | expression | semmle.label | expression | | JakartaExpressionInjection.java:61:24:61:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:65:13:65:13 | e | semmle.label | e | -| JakartaExpressionInjection.java:70:24:70:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:74:13:74:13 | e | semmle.label | e | -| JakartaExpressionInjection.java:79:24:79:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:83:13:83:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:63:43:63:52 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:68:24:68:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:72:13:72:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:77:24:77:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:81:13:81:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:86:24:86:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:90:13:90:13 | e | semmle.label | e | #select | JakartaExpressionInjection.java:32:28:32:37 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:32:28:32:37 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | | JakartaExpressionInjection.java:39:32:39:41 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:39:32:39:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | | JakartaExpressionInjection.java:56:32:56:41 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:56:32:56:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:65:13:65:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:65:13:65:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:74:13:74:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:74:13:74:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:83:13:83:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:83:13:83:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:63:43:63:52 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:63:43:63:52 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:72:13:72:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:72:13:72:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:81:13:81:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:81:13:81:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:90:13:90:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:90:13:90:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java index a9fb2d45d6f4..2e1d1e55b020 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java @@ -57,6 +57,13 @@ private static void testWithELProcessorSetValue() throws IOException { }); } + private static void testWithELProcessorSetVariable() throws IOException { + testWithSocket(expression -> { + ELProcessor processor = new ELProcessor(); + processor.setVariable("test", expression); + }); + } + private static void testWithJuelValueExpressionGetValue() throws IOException { testWithSocket(expression -> { ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); diff --git a/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java b/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java index 3c523e685c5e..95895fabc648 100644 --- a/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java +++ b/java/ql/test/stubs/java-ee-el/javax/el/ELProcessor.java @@ -4,4 +4,5 @@ public class ELProcessor { public Object eval(String expression) { return null; } public Object getValue(String expression, Class expectedType) { return null; } public void setValue(String expression, Object value) {} + public void setVariable(String var, String expression) {} } From b96b6652626cbeb232748ac782130b884335725d Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Mon, 12 Apr 2021 21:40:49 +0300 Subject: [PATCH 09/10] Renaming in java/ql/src/experimental/Security/CWE/CWE-094 --- .../CWE/CWE-094/{InjectionLib.qll => FlowUtils.qll} | 2 +- .../Security/CWE/CWE-094/JakartaExpressionInjection.qhelp | 6 +++--- .../Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll | 4 ++-- .../experimental/Security/CWE/CWE-094/JexlInjectionLib.qll | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename java/ql/src/experimental/Security/CWE/CWE-094/{InjectionLib.qll => FlowUtils.qll} (81%) diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/FlowUtils.qll similarity index 81% rename from java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll rename to java/ql/src/experimental/Security/CWE/CWE-094/FlowUtils.qll index adab79d6f5c3..5371c51aa8f0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/InjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/FlowUtils.qll @@ -5,7 +5,7 @@ import semmle.code.java.dataflow.FlowSources * Holds if `fromNode` to `toNode` is a dataflow step that returns data from * a bean by calling one of its getters. */ -predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node toNode) { +predicate hasGetterFlow(DataFlow::Node fromNode, DataFlow::Node toNode) { exists(MethodAccess ma, Method m | ma.getMethod() = m | m instanceof GetterMethod and ma.getQualifier() = fromNode.asExpr() and diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp index a6d66c1d1d16..a8d3cd0fe70e 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp @@ -29,14 +29,14 @@ with sandboxing capabilities such as Apache Commons JEXL or the Spring Expressio The following example shows how untrusted data is used to build and run an expression using the JUEL interpreter:

    - +

    -JUEL does not support to run expressions in a sandbox. To prevent running arbitrary code, +JUEL does not support running expressions in a sandbox. To prevent running arbitrary code, incoming data has to be checked before including it in an expression. The next example uses a Regex pattern to check whether a user tries to run an allowed expression or not:

    - + diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll index b1c5d1e8ae20..430909743647 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjectionLib.qll @@ -1,5 +1,5 @@ import java -import InjectionLib +import FlowUtils import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking @@ -16,7 +16,7 @@ class JakartaExpressionInjectionConfig extends TaintTracking::Configuration { override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { any(TaintPropagatingCall c).taintFlow(fromNode, toNode) or - returnsDataFromBean(fromNode, toNode) + hasGetterFlow(fromNode, toNode) } } diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll index 51084a9862ce..89d7cb496a41 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/JexlInjectionLib.qll @@ -1,5 +1,5 @@ import java -import InjectionLib +import FlowUtils import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking @@ -17,7 +17,7 @@ class JexlInjectionConfig extends TaintTracking::Configuration { override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { any(TaintPropagatingJexlMethodCall c).taintFlow(fromNode, toNode) or - returnsDataFromBean(fromNode, toNode) + hasGetterFlow(fromNode, toNode) } } From 97186b3d30bcfc095cd4f30cc407330c094687a6 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 14 Apr 2021 19:30:58 +0300 Subject: [PATCH 10/10] Added comments for tests --- .../JakartaExpressionInjection.expected | 86 +++++++++---------- .../CWE-094/JakartaExpressionInjection.java | 9 ++ 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected index ef002aefdf5c..390871b54cf1 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected @@ -1,46 +1,46 @@ edges -| JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:24:31:24:40 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:30:24:30:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:37:24:37:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:44:24:44:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:54:24:54:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:61:24:61:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:68:24:68:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:77:24:77:33 | expression : String | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | JakartaExpressionInjection.java:86:24:86:33 | expression : String | -| JakartaExpressionInjection.java:30:24:30:33 | expression : String | JakartaExpressionInjection.java:32:28:32:37 | expression | -| JakartaExpressionInjection.java:37:24:37:33 | expression : String | JakartaExpressionInjection.java:39:32:39:41 | expression | -| JakartaExpressionInjection.java:44:24:44:33 | expression : String | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | -| JakartaExpressionInjection.java:54:24:54:33 | expression : String | JakartaExpressionInjection.java:56:32:56:41 | expression | -| JakartaExpressionInjection.java:61:24:61:33 | expression : String | JakartaExpressionInjection.java:63:43:63:52 | expression | -| JakartaExpressionInjection.java:68:24:68:33 | expression : String | JakartaExpressionInjection.java:72:13:72:13 | e | -| JakartaExpressionInjection.java:77:24:77:33 | expression : String | JakartaExpressionInjection.java:81:13:81:13 | e | -| JakartaExpressionInjection.java:86:24:86:33 | expression : String | JakartaExpressionInjection.java:90:13:90:13 | e | +| JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:25:31:25:40 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:32:24:32:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:40:24:40:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:48:24:48:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:59:24:59:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:67:24:67:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:75:24:75:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:85:24:85:33 | expression : String | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:95:24:95:33 | expression : String | +| JakartaExpressionInjection.java:32:24:32:33 | expression : String | JakartaExpressionInjection.java:34:28:34:37 | expression | +| JakartaExpressionInjection.java:40:24:40:33 | expression : String | JakartaExpressionInjection.java:42:32:42:41 | expression | +| JakartaExpressionInjection.java:48:24:48:33 | expression : String | JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | +| JakartaExpressionInjection.java:59:24:59:33 | expression : String | JakartaExpressionInjection.java:61:32:61:41 | expression | +| JakartaExpressionInjection.java:67:24:67:33 | expression : String | JakartaExpressionInjection.java:69:43:69:52 | expression | +| JakartaExpressionInjection.java:75:24:75:33 | expression : String | JakartaExpressionInjection.java:79:13:79:13 | e | +| JakartaExpressionInjection.java:85:24:85:33 | expression : String | JakartaExpressionInjection.java:89:13:89:13 | e | +| JakartaExpressionInjection.java:95:24:95:33 | expression : String | JakartaExpressionInjection.java:99:13:99:13 | e | nodes -| JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| JakartaExpressionInjection.java:24:31:24:40 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:30:24:30:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:32:28:32:37 | expression | semmle.label | expression | -| JakartaExpressionInjection.java:37:24:37:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:39:32:39:41 | expression | semmle.label | expression | -| JakartaExpressionInjection.java:44:24:44:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | semmle.label | lambdaExpression | -| JakartaExpressionInjection.java:54:24:54:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:56:32:56:41 | expression | semmle.label | expression | -| JakartaExpressionInjection.java:61:24:61:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:63:43:63:52 | expression | semmle.label | expression | -| JakartaExpressionInjection.java:68:24:68:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:72:13:72:13 | e | semmle.label | e | -| JakartaExpressionInjection.java:77:24:77:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:81:13:81:13 | e | semmle.label | e | -| JakartaExpressionInjection.java:86:24:86:33 | expression : String | semmle.label | expression : String | -| JakartaExpressionInjection.java:90:13:90:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | +| JakartaExpressionInjection.java:25:31:25:40 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:32:24:32:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:34:28:34:37 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:40:24:40:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:42:32:42:41 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:48:24:48:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | semmle.label | lambdaExpression | +| JakartaExpressionInjection.java:59:24:59:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:61:32:61:41 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:67:24:67:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:69:43:69:52 | expression | semmle.label | expression | +| JakartaExpressionInjection.java:75:24:75:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:79:13:79:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:85:24:85:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:89:13:89:13 | e | semmle.label | e | +| JakartaExpressionInjection.java:95:24:95:33 | expression : String | semmle.label | expression : String | +| JakartaExpressionInjection.java:99:13:99:13 | e | semmle.label | e | #select -| JakartaExpressionInjection.java:32:28:32:37 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:32:28:32:37 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:39:32:39:41 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:39:32:39:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:49:13:49:28 | lambdaExpression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:56:32:56:41 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:56:32:56:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:63:43:63:52 | expression | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:63:43:63:52 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:72:13:72:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:72:13:72:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:81:13:81:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:81:13:81:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | -| JakartaExpressionInjection.java:90:13:90:13 | e | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:90:13:90:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:22:25:22:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:34:28:34:37 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:34:28:34:37 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:42:32:42:41 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:42:32:42:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:61:32:61:41 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:61:32:61:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:69:43:69:52 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:69:43:69:52 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:79:13:79:13 | e | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:79:13:79:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:89:13:89:13 | e | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:89:13:89:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | +| JakartaExpressionInjection.java:99:13:99:13 | e | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:99:13:99:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java index 2e1d1e55b020..ae5b6a8d5e41 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java +++ b/java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.java @@ -15,6 +15,7 @@ public class JakartaExpressionInjection { + // calls a consumer with a string received from a socket private static void testWithSocket(Consumer action) throws IOException { try (ServerSocket serverSocket = new ServerSocket(0)) { try (Socket socket = serverSocket.accept()) { @@ -26,6 +27,7 @@ private static void testWithSocket(Consumer action) throws IOException { } } + // BAD (untrusted input to ELProcessor.eval) private static void testWithELProcessorEval() throws IOException { testWithSocket(expression -> { ELProcessor processor = new ELProcessor(); @@ -33,6 +35,7 @@ private static void testWithELProcessorEval() throws IOException { }); } + // BAD (untrusted input to ELProcessor.getValue) private static void testWithELProcessorGetValue() throws IOException { testWithSocket(expression -> { ELProcessor processor = new ELProcessor(); @@ -40,6 +43,7 @@ private static void testWithELProcessorGetValue() throws IOException { }); } + // BAD (untrusted input to LambdaExpression.invoke) private static void testWithLambdaExpressionInvoke() throws IOException { testWithSocket(expression -> { ExpressionFactory factory = ELManager.getExpressionFactory(); @@ -50,6 +54,7 @@ private static void testWithLambdaExpressionInvoke() throws IOException { }); } + // BAD (untrusted input to ELProcessor.setValue) private static void testWithELProcessorSetValue() throws IOException { testWithSocket(expression -> { ELProcessor processor = new ELProcessor(); @@ -57,6 +62,7 @@ private static void testWithELProcessorSetValue() throws IOException { }); } + // BAD (untrusted input to ELProcessor.setVariable) private static void testWithELProcessorSetVariable() throws IOException { testWithSocket(expression -> { ELProcessor processor = new ELProcessor(); @@ -64,6 +70,7 @@ private static void testWithELProcessorSetVariable() throws IOException { }); } + // BAD (untrusted input to ValueExpression.getValue when it was created by JUEL) private static void testWithJuelValueExpressionGetValue() throws IOException { testWithSocket(expression -> { ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); @@ -73,6 +80,7 @@ private static void testWithJuelValueExpressionGetValue() throws IOException { }); } + // BAD (untrusted input to ValueExpression.setValue when it was created by JUEL) private static void testWithJuelValueExpressionSetValue() throws IOException { testWithSocket(expression -> { ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl(); @@ -82,6 +90,7 @@ private static void testWithJuelValueExpressionSetValue() throws IOException { }); } + // BAD (untrusted input to MethodExpression.invoke when it was created by JUEL) private static void testWithJuelMethodExpressionInvoke() throws IOException { testWithSocket(expression -> { ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl();