Merge pull request #6037 from atorralba/atorralba/promote-spel-injection

Java: Promote SpEL Injection query from experimental

java/change-notes/2021-06-08-spel-injection-query.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The query "Expression language injection (Spring)" (`java/spel-expression-injection`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @artem-smotrakov](https://github.com/github/codeql/pull/3291).

java/ql/lib/semmle/code/java/frameworks/spring/SpringExpression.qll

@@ -0,0 +1,48 @@
+/**
+ * Provides classes for working with the Spring Expression Language (SpEL).
+ */
+
+import java
+
+/**
+ * Methods that trigger the evaluation of a SpEL expression.
+ */
+class ExpressionEvaluationMethod extends Method {
+  ExpressionEvaluationMethod() {
+    this.getDeclaringType().getASupertype*() instanceof Expression and
+    this.hasName(["getValue", "getValueTypeDescriptor", "getValueType", "setValue"])
+  }
+}
+
+/**
+ * The class `org.springframework.expression.ExpressionParser`.
+ */
+class ExpressionParser extends RefType {
+  ExpressionParser() { hasQualifiedName("org.springframework.expression", "ExpressionParser") }
+}
+
+/**
+ * The class `org.springframework.expression.spel.support.SimpleEvaluationContext$Builder`.
+ */
+class SimpleEvaluationContextBuilder extends RefType {
+  SimpleEvaluationContextBuilder() {
+    hasQualifiedName("org.springframework.expression.spel.support",
+      "SimpleEvaluationContext$Builder")
+  }
+}
+
+/**
+ * The class `org.springframework.expression.Expression`.
+ */
+class Expression extends RefType {
+  Expression() { hasQualifiedName("org.springframework.expression", "Expression") }
+}
+
+/**
+ * The class `org.springframework.expression.spel.support.SimpleEvaluationContext`.
+ */
+class SimpleEvaluationContext extends RefType {
+  SimpleEvaluationContext() {
+    hasQualifiedName("org.springframework.expression.spel.support", "SimpleEvaluationContext")
+  }
+}

java/ql/lib/semmle/code/java/security/SpelInjection.qll

@@ -0,0 +1,42 @@
+/** Provides classes to reason about SpEL injection attacks. */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.frameworks.spring.SpringExpression
+
+/** A data flow sink for unvalidated user input that is used to construct SpEL expressions. */
+abstract class SpelExpressionEvaluationSink extends DataFlow::ExprNode { }
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to the `SpELInjectionConfig`.
+ */
+class SpelExpressionInjectionAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for the `SpELInjectionConfig` configuration.
+   */
+  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+}
+
+/** A set of additional taint steps to consider when taint tracking SpEL related data flows. */
+private class DefaultSpelExpressionInjectionAdditionalTaintStep extends SpelExpressionInjectionAdditionalTaintStep {
+  override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+    expressionParsingStep(node1, node2)
+  }
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that parses a SpEL expression,
+ * by calling `parser.parseExpression(tainted)`.
+ */
+private predicate expressionParsingStep(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    m.getDeclaringType().getASupertype*() instanceof ExpressionParser and
+    m.hasName(["parseExpression", "parseRaw"]) and
+    ma.getAnArgument() = node1.asExpr() and
+    node2.asExpr() = ma
+  )
+}

java/ql/lib/semmle/code/java/security/SpelInjectionQuery.qll

@@ -0,0 +1,86 @@
+/** Provides taint tracking and dataflow configurations to be used in SpEL injection queries. */
+
+import java
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.frameworks.spring.SpringExpression
+private import semmle.code.java.security.SpelInjection
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a SpEL expression.
+ */
+class SpelInjectionConfig extends TaintTracking::Configuration {
+  SpelInjectionConfig() { this = "SpelInjectionConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/** Default sink for SpEL injection vulnerabilities. */
+private class DefaultSpelExpressionEvaluationSink extends SpelExpressionEvaluationSink {
+  DefaultSpelExpressionEvaluationSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof ExpressionEvaluationMethod and
+      ma.getQualifier() = this.asExpr() and
+      not exists(SafeEvaluationContextFlowConfig config |
+        config.hasFlowTo(DataFlow::exprNode(ma.getArgument(0)))
+      )
+    )
+  }
+}
+
+/**
+ * A configuration for safe evaluation context that may be used in expression evaluation.
+ */
+private class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration {
+  SafeEvaluationContextFlowConfig() { this = "SpelInjection::SafeEvaluationContextFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof ExpressionEvaluationMethod and
+      ma.getArgument(0) = sink.asExpr()
+    )
+  }
+
+  override int fieldFlowBranchLimit() { result = 0 }
+}
+
+/**
+ * A `ContextSource` that is safe from SpEL injection.
+ */
+private class SafeContextSource extends DataFlow::ExprNode {
+  SafeContextSource() {
+    isSimpleEvaluationContextConstructorCall(getExpr()) or
+    isSimpleEvaluationContextBuilderCall(getExpr())
+  }
+}
+
+/**
+ * Holds if `expr` constructs `SimpleEvaluationContext`.
+ */
+private predicate isSimpleEvaluationContextConstructorCall(Expr expr) {
+  exists(ConstructorCall cc |
+    cc.getConstructedType() instanceof SimpleEvaluationContext and
+    cc = expr
+  )
+}
+
+/**
+ * Holds if `expr` builds `SimpleEvaluationContext` via `SimpleEvaluationContext.Builder`,
+ * for instance, `SimpleEvaluationContext.forReadWriteDataBinding().build()`.
+ */
+private predicate isSimpleEvaluationContextBuilderCall(Expr expr) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    m.getDeclaringType() instanceof SimpleEvaluationContextBuilder and
+    m.hasName("build") and
+    ma = expr
+  )
+}

java/ql/src/Security/CWE/CWE-094/SpelInjection.qhelp

@@ -4,7 +4,7 @@
 <overview>
 <p>
 The Spring Expression Language (SpEL) is a powerful expression language
-provided by Spring Framework. The language offers many features
+provided by the Spring Framework. The language offers many features
 including invocation of methods available in the JVM.
 If a SpEL expression is built using attacker-controlled data, 
 and then evaluated in a powerful context,
@@ -31,7 +31,7 @@ that doesn't allow arbitrary method invocation.
 <example>
 <p>
 The following example uses untrusted data to build a SpEL expression
-and then runs it in the default powerfull context.
+and then runs it in the default powerful context.
 </p>
 <sample src="UnsafeSpelExpressionEvaluation.java" />
 
@@ -53,4 +53,4 @@ However, it's recommended to avoid using untrusted input in SpEL expressions.
   <a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
 </li>
 </references>
-</qhelp>
+</qhelp>

java/ql/src/Security/CWE/CWE-094/SpelInjection.ql

@@ -11,9 +11,10 @@
  */
 
 import java
-import SpelInjectionLib
+import semmle.code.java.security.SpelInjectionQuery
+import semmle.code.java.dataflow.DataFlow
 import DataFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, ExpressionInjectionConfig conf
+from DataFlow::PathNode source, DataFlow::PathNode sink, SpelInjectionConfig conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "SpEL injection from $@.", source.getNode(), "this user input"

java/ql/src/experimental/Security/CWE/CWE-094/SpelInjectionLib.qll

@@ -1,100 +0,0 @@
-import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking2
-import SpringFrameworkLib
-
-/**
- * A taint-tracking configuration for unsafe user input
- * that is used to construct and evaluate a SpEL expression.
- */
-class ExpressionInjectionConfig extends TaintTracking::Configuration {
-  ExpressionInjectionConfig() { this = "ExpressionInjectionConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
-    source instanceof RemoteFlowSource or
-    source instanceof WebRequestSource
-  }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    expressionParsingStep(node1, node2) or
-    springPropertiesStep(node1, node2)
-  }
-}
-
-/**
- * A sink for SpEL injection vulnerabilities,
- * i.e. methods that run evaluation of a SpEL expression in a powerfull context.
- */
-class ExpressionEvaluationSink extends DataFlow::ExprNode {
-  ExpressionEvaluationSink() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      m instanceof ExpressionEvaluationMethod and
-      getExpr() = ma.getQualifier() and
-      not exists(SafeEvaluationContextFlowConfig config |
-        config.hasFlowTo(DataFlow::exprNode(ma.getArgument(0)))
-      )
-    )
-  }
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that parses a SpEL expression,
- * i.e. `parser.parseExpression(tainted)`.
- */
-predicate expressionParsingStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(MethodAccess ma, Method m | ma.getMethod() = m |
-    m.getDeclaringType().getAnAncestor*() instanceof ExpressionParser and
-    m.hasName("parseExpression") and
-    ma.getAnArgument() = node1.asExpr() and
-    node2.asExpr() = ma
-  )
-}
-
-/**
- * A configuration for safe evaluation context that may be used in expression evaluation.
- */
-class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration {
-  SafeEvaluationContextFlowConfig() { this = "SpelInjection::SafeEvaluationContextFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
-
-  override predicate isSink(DataFlow::Node sink) {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      m instanceof ExpressionEvaluationMethod and
-      ma.getArgument(0) = sink.asExpr()
-    )
-  }
-
-  override int fieldFlowBranchLimit() { result = 0 }
-}
-
-class SafeContextSource extends DataFlow::ExprNode {
-  SafeContextSource() {
-    isSimpleEvaluationContextConstructorCall(getExpr()) or
-    isSimpleEvaluationContextBuilderCall(getExpr())
-  }
-}
-
-/**
- * Holds if `expr` constructs `SimpleEvaluationContext`.
- */
-predicate isSimpleEvaluationContextConstructorCall(Expr expr) {
-  exists(ConstructorCall cc |
-    cc.getConstructedType() instanceof SimpleEvaluationContext and
-    cc = expr
-  )
-}
-
-/**
- * Holds if `expr` builds `SimpleEvaluationContext` via `SimpleEvaluationContext.Builder`,
- * e.g. `SimpleEvaluationContext.forReadWriteDataBinding().build()`.
- */
-predicate isSimpleEvaluationContextBuilderCall(Expr expr) {
-  exists(MethodAccess ma, Method m | ma.getMethod() = m |
-    m.getDeclaringType() instanceof SimpleEvaluationContextBuilder and
-    m.hasName("build") and
-    ma = expr
-  )
-}

java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll

@@ -1,21 +1,6 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 
-/**
- * Methods that trigger evaluation of an expression.
- */
-class ExpressionEvaluationMethod extends Method {
-  ExpressionEvaluationMethod() {
-    getDeclaringType() instanceof Expression and
-    (
-      hasName("getValue") or
-      hasName("getValueTypeDescriptor") or
-      hasName("getValueType") or
-      hasName("setValue")
-    )
-  }
-}
-
 /**
  * `WebRequest` interface is a source of tainted data.
  */
@@ -37,100 +22,6 @@ class WebRequestSource extends DataFlow::Node {
   }
 }
 
-/**
- * Holds if `node1` to `node2` is a dataflow step that converts `PropertyValues`
- * to an array of `PropertyValue`, i.e. `tainted.getPropertyValues()`.
- */
-predicate getPropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(MethodAccess ma, Method m | m = ma.getMethod() |
-    node1.asExpr() = ma.getQualifier() and
-    node2.asExpr() = ma and
-    m.getDeclaringType() instanceof PropertyValues and
-    m.hasName("getPropertyValues")
-  )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that constructs `MutablePropertyValues`,
- * i.e. `new MutablePropertyValues(tainted)`.
- */
-predicate createMutablePropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(ConstructorCall cc | cc.getConstructedType() instanceof MutablePropertyValues |
-    node1.asExpr() = cc.getAnArgument() and
-    node2.asExpr() = cc
-  )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that returns a name of `PropertyValue`,
- * i.e. `tainted.getName()`.
- */
-predicate getPropertyNameStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(MethodAccess ma, Method m | m = ma.getMethod() |
-    node1.asExpr() = ma.getQualifier() and
-    node2.asExpr() = ma and
-    m.getDeclaringType() instanceof PropertyValue and
-    m.hasName("getName")
-  )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that converts `MutablePropertyValues`
- * to a list of `PropertyValue`, i.e. `tainted.getPropertyValueList()`.
- */
-predicate getPropertyValueListStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(MethodAccess ma, Method m | m = ma.getMethod() |
-    node1.asExpr() = ma.getQualifier() and
-    node2.asExpr() = ma and
-    m.getDeclaringType() instanceof MutablePropertyValues and
-    m.hasName("getPropertyValueList")
-  )
-}
-
-/**
- * Holds if `node1` to `node2` is one of the dataflow steps that propagate
- * tainted data via Spring properties.
- */
-predicate springPropertiesStep(DataFlow::Node node1, DataFlow::Node node2) {
-  createMutablePropertyValuesStep(node1, node2) or
-  getPropertyNameStep(node1, node2) or
-  getPropertyValuesStep(node1, node2) or
-  getPropertyValueListStep(node1, node2)
-}
-
-class PropertyValue extends RefType {
-  PropertyValue() { hasQualifiedName("org.springframework.beans", "PropertyValue") }
-}
-
-class PropertyValues extends RefType {
-  PropertyValues() { hasQualifiedName("org.springframework.beans", "PropertyValues") }
-}
-
-class MutablePropertyValues extends RefType {
-  MutablePropertyValues() { hasQualifiedName("org.springframework.beans", "MutablePropertyValues") }
-}
-
-class SimpleEvaluationContext extends RefType {
-  SimpleEvaluationContext() {
-    hasQualifiedName("org.springframework.expression.spel.support", "SimpleEvaluationContext")
-  }
-}
-
-class SimpleEvaluationContextBuilder extends RefType {
-  SimpleEvaluationContextBuilder() {
-    hasQualifiedName("org.springframework.expression.spel.support",
-      "SimpleEvaluationContext$Builder")
-  }
-}
-
 class WebRequest extends RefType {
   WebRequest() { hasQualifiedName("org.springframework.web.context.request", "WebRequest") }
 }
-
-class Expression extends RefType {
-  Expression() { hasQualifiedName("org.springframework.expression", "Expression") }
-}
-
-class ExpressionParser extends RefType {
-  ExpressionParser() { hasQualifiedName("org.springframework.expression", "ExpressionParser") }
-}

java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.expected

@@ -1,76 +0,0 @@
-edges
-| SpelInjection.java:15:22:15:44 | getInputStream(...) : InputStream | SpelInjection.java:18:13:18:14 | in : InputStream |
-| SpelInjection.java:18:13:18:14 | in : InputStream | SpelInjection.java:18:21:18:25 | bytes [post update] : byte[] |
-| SpelInjection.java:18:21:18:25 | bytes [post update] : byte[] | SpelInjection.java:19:31:19:35 | bytes : byte[] |
-| SpelInjection.java:19:20:19:42 | new String(...) : String | SpelInjection.java:23:5:23:14 | expression |
-| SpelInjection.java:19:31:19:35 | bytes : byte[] | SpelInjection.java:19:20:19:42 | new String(...) : String |
-| SpelInjection.java:27:22:27:44 | getInputStream(...) : InputStream | SpelInjection.java:30:13:30:14 | in : InputStream |
-| SpelInjection.java:30:13:30:14 | in : InputStream | SpelInjection.java:30:21:30:25 | bytes [post update] : byte[] |
-| SpelInjection.java:30:21:30:25 | bytes [post update] : byte[] | SpelInjection.java:31:31:31:35 | bytes : byte[] |
-| SpelInjection.java:31:20:31:42 | new String(...) : String | SpelInjection.java:34:5:34:14 | expression |
-| SpelInjection.java:31:31:31:35 | bytes : byte[] | SpelInjection.java:31:20:31:42 | new String(...) : String |
-| SpelInjection.java:38:22:38:44 | getInputStream(...) : InputStream | SpelInjection.java:41:13:41:14 | in : InputStream |
-| SpelInjection.java:41:13:41:14 | in : InputStream | SpelInjection.java:41:21:41:25 | bytes [post update] : byte[] |
-| SpelInjection.java:41:21:41:25 | bytes [post update] : byte[] | SpelInjection.java:42:31:42:35 | bytes : byte[] |
-| SpelInjection.java:42:20:42:42 | new String(...) : String | SpelInjection.java:48:5:48:14 | expression |
-| SpelInjection.java:42:31:42:35 | bytes : byte[] | SpelInjection.java:42:20:42:42 | new String(...) : String |
-| SpelInjection.java:52:22:52:44 | getInputStream(...) : InputStream | SpelInjection.java:55:13:55:14 | in : InputStream |
-| SpelInjection.java:55:13:55:14 | in : InputStream | SpelInjection.java:55:21:55:25 | bytes [post update] : byte[] |
-| SpelInjection.java:55:21:55:25 | bytes [post update] : byte[] | SpelInjection.java:56:31:56:35 | bytes : byte[] |
-| SpelInjection.java:56:20:56:42 | new String(...) : String | SpelInjection.java:59:5:59:14 | expression |
-| SpelInjection.java:56:31:56:35 | bytes : byte[] | SpelInjection.java:56:20:56:42 | new String(...) : String |
-| SpelInjection.java:63:22:63:44 | getInputStream(...) : InputStream | SpelInjection.java:66:13:66:14 | in : InputStream |
-| SpelInjection.java:66:13:66:14 | in : InputStream | SpelInjection.java:66:21:66:25 | bytes [post update] : byte[] |
-| SpelInjection.java:66:21:66:25 | bytes [post update] : byte[] | SpelInjection.java:67:31:67:35 | bytes : byte[] |
-| SpelInjection.java:67:20:67:42 | new String(...) : String | SpelInjection.java:70:5:70:14 | expression |
-| SpelInjection.java:67:31:67:35 | bytes : byte[] | SpelInjection.java:67:20:67:42 | new String(...) : String |
-| SpelInjection.java:74:22:74:44 | getInputStream(...) : InputStream | SpelInjection.java:77:13:77:14 | in : InputStream |
-| SpelInjection.java:77:13:77:14 | in : InputStream | SpelInjection.java:77:21:77:25 | bytes [post update] : byte[] |
-| SpelInjection.java:77:21:77:25 | bytes [post update] : byte[] | SpelInjection.java:78:31:78:35 | bytes : byte[] |
-| SpelInjection.java:78:20:78:42 | new String(...) : String | SpelInjection.java:83:5:83:14 | expression |
-| SpelInjection.java:78:31:78:35 | bytes : byte[] | SpelInjection.java:78:20:78:42 | new String(...) : String |
-nodes
-| SpelInjection.java:15:22:15:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| SpelInjection.java:18:13:18:14 | in : InputStream | semmle.label | in : InputStream |
-| SpelInjection.java:18:21:18:25 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| SpelInjection.java:19:20:19:42 | new String(...) : String | semmle.label | new String(...) : String |
-| SpelInjection.java:19:31:19:35 | bytes : byte[] | semmle.label | bytes : byte[] |
-| SpelInjection.java:23:5:23:14 | expression | semmle.label | expression |
-| SpelInjection.java:27:22:27:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| SpelInjection.java:30:13:30:14 | in : InputStream | semmle.label | in : InputStream |
-| SpelInjection.java:30:21:30:25 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| SpelInjection.java:31:20:31:42 | new String(...) : String | semmle.label | new String(...) : String |
-| SpelInjection.java:31:31:31:35 | bytes : byte[] | semmle.label | bytes : byte[] |
-| SpelInjection.java:34:5:34:14 | expression | semmle.label | expression |
-| SpelInjection.java:38:22:38:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| SpelInjection.java:41:13:41:14 | in : InputStream | semmle.label | in : InputStream |
-| SpelInjection.java:41:21:41:25 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| SpelInjection.java:42:20:42:42 | new String(...) : String | semmle.label | new String(...) : String |
-| SpelInjection.java:42:31:42:35 | bytes : byte[] | semmle.label | bytes : byte[] |
-| SpelInjection.java:48:5:48:14 | expression | semmle.label | expression |
-| SpelInjection.java:52:22:52:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| SpelInjection.java:55:13:55:14 | in : InputStream | semmle.label | in : InputStream |
-| SpelInjection.java:55:21:55:25 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| SpelInjection.java:56:20:56:42 | new String(...) : String | semmle.label | new String(...) : String |
-| SpelInjection.java:56:31:56:35 | bytes : byte[] | semmle.label | bytes : byte[] |
-| SpelInjection.java:59:5:59:14 | expression | semmle.label | expression |
-| SpelInjection.java:63:22:63:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| SpelInjection.java:66:13:66:14 | in : InputStream | semmle.label | in : InputStream |
-| SpelInjection.java:66:21:66:25 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| SpelInjection.java:67:20:67:42 | new String(...) : String | semmle.label | new String(...) : String |
-| SpelInjection.java:67:31:67:35 | bytes : byte[] | semmle.label | bytes : byte[] |
-| SpelInjection.java:70:5:70:14 | expression | semmle.label | expression |
-| SpelInjection.java:74:22:74:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| SpelInjection.java:77:13:77:14 | in : InputStream | semmle.label | in : InputStream |
-| SpelInjection.java:77:21:77:25 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| SpelInjection.java:78:20:78:42 | new String(...) : String | semmle.label | new String(...) : String |
-| SpelInjection.java:78:31:78:35 | bytes : byte[] | semmle.label | bytes : byte[] |
-| SpelInjection.java:83:5:83:14 | expression | semmle.label | expression |
-subpaths
-#select
-| SpelInjection.java:23:5:23:14 | expression | SpelInjection.java:15:22:15:44 | getInputStream(...) : InputStream | SpelInjection.java:23:5:23:14 | expression | SpEL injection from $@. | SpelInjection.java:15:22:15:44 | getInputStream(...) | this user input |
-| SpelInjection.java:34:5:34:14 | expression | SpelInjection.java:27:22:27:44 | getInputStream(...) : InputStream | SpelInjection.java:34:5:34:14 | expression | SpEL injection from $@. | SpelInjection.java:27:22:27:44 | getInputStream(...) | this user input |
-| SpelInjection.java:48:5:48:14 | expression | SpelInjection.java:38:22:38:44 | getInputStream(...) : InputStream | SpelInjection.java:48:5:48:14 | expression | SpEL injection from $@. | SpelInjection.java:38:22:38:44 | getInputStream(...) | this user input |
-| SpelInjection.java:59:5:59:14 | expression | SpelInjection.java:52:22:52:44 | getInputStream(...) : InputStream | SpelInjection.java:59:5:59:14 | expression | SpEL injection from $@. | SpelInjection.java:52:22:52:44 | getInputStream(...) | this user input |
-| SpelInjection.java:70:5:70:14 | expression | SpelInjection.java:63:22:63:44 | getInputStream(...) : InputStream | SpelInjection.java:70:5:70:14 | expression | SpEL injection from $@. | SpelInjection.java:63:22:63:44 | getInputStream(...) | this user input |
-| SpelInjection.java:83:5:83:14 | expression | SpelInjection.java:74:22:74:44 | getInputStream(...) : InputStream | SpelInjection.java:83:5:83:14 | expression | SpEL injection from $@. | SpelInjection.java:74:22:74:44 | getInputStream(...) | this user input |

java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.qlref

@@ -1 +0,0 @@
-experimental/Security/CWE/CWE-094/SpelInjection.ql

java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.java

@@ -3,11 +3,12 @@ import java.io.InputStream;
 import java.net.Socket;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ExpressionParser;
+import org.springframework.expression.spel.standard.SpelExpression;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.SimpleEvaluationContext;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 
-public class SpelInjection {
+public class SpelInjectionTest {
 
   private static final ExpressionParser PARSER = new SpelExpressionParser();
 
@@ -20,7 +21,18 @@ public class SpelInjection {
 
     ExpressionParser parser = new SpelExpressionParser();
     Expression expression = parser.parseExpression(input);
-    expression.getValue();
+    expression.getValue(); // $hasSpelInjection
+  }
+
+  public void testGetValueWithParseRaw(Socket socket) throws IOException {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+    SpelExpressionParser parser = new SpelExpressionParser();
+    SpelExpression expression = parser.parseRaw(input);
+    expression.getValue(); // $hasSpelInjection
   }
 
   public void testGetValueWithChainedCalls(Socket socket) throws IOException {
@@ -31,7 +43,7 @@ public class SpelInjection {
     String input = new String(bytes, 0, n);
 
     Expression expression = new SpelExpressionParser().parseExpression(input);
-    expression.getValue();
+    expression.getValue(); // $hasSpelInjection
   }
 
   public void testSetValueWithRootObject(Socket socket) throws IOException {
@@ -45,7 +57,7 @@ public class SpelInjection {
 
     Object root = new Object();
     Object value = new Object();
-    expression.setValue(root, value);
+    expression.setValue(root, value); // $hasSpelInjection
   }
 
   public void testGetValueWithStaticParser(Socket socket) throws IOException {
@@ -56,7 +68,7 @@ public class SpelInjection {
     String input = new String(bytes, 0, n);
 
     Expression expression = PARSER.parseExpression(input);
-    expression.getValue();
+    expression.getValue(); // $hasSpelInjection
   }
 
   public void testGetValueType(Socket socket) throws IOException {
@@ -67,7 +79,7 @@ public class SpelInjection {
     String input = new String(bytes, 0, n);
 
     Expression expression = PARSER.parseExpression(input);
-    expression.getValueType();
+    expression.getValueType(); // $hasSpelInjection
   }
 
   public void testWithStandardEvaluationContext(Socket socket) throws IOException {
@@ -80,7 +92,7 @@ public class SpelInjection {
     Expression expression = PARSER.parseExpression(input);
 
     StandardEvaluationContext context = new StandardEvaluationContext();
-    expression.getValue(context);
+    expression.getValue(context); // $hasSpelInjection
   }
 
   public void testWithSimpleEvaluationContext(Socket socket) throws IOException {
@@ -93,8 +105,7 @@ public class SpelInjection {
     Expression expression = PARSER.parseExpression(input);
     SimpleEvaluationContext context = SimpleEvaluationContext.forReadWriteDataBinding().build();
 
-    // the expression is evaluated in a limited context
-    expression.getValue(context);
+    expression.getValue(context); // Safe - the expression is evaluated in a limited context
   }
 
 }

java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql

@@ -0,0 +1,22 @@
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.SpelInjectionQuery
+import TestUtilities.InlineExpectationsTest
+
+class HasSpelInjectionTest extends InlineExpectationsTest {
+  HasSpelInjectionTest() { this = "HasSpelInjectionTest" }
+
+  override string getARelevantTag() { result = "hasSpelInjection" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasSpelInjection" and
+    exists(DataFlow::Node src, DataFlow::Node sink, SpelInjectionConfig conf |
+      conf.hasFlow(src, sink)
+    |
+      sink.getLocation() = location and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/Expression.java

@@ -1,14 +1,65 @@
+/*
+ * Copyright 2002-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
 package org.springframework.expression;
 
+import org.springframework.lang.Nullable;
+
 public interface Expression {
+	String getExpressionString();
 
 	Object getValue() throws EvaluationException;
 
+	<T> T getValue(@Nullable Class<T> desiredResultType) throws EvaluationException;
+
+	Object getValue(@Nullable Object rootObject) throws EvaluationException;
+
+	<T> T getValue(@Nullable Object rootObject, @Nullable Class<T> desiredResultType)
+			throws EvaluationException;
+
 	Object getValue(EvaluationContext context) throws EvaluationException;
 
+	Object getValue(EvaluationContext context, @Nullable Object rootObject)
+			throws EvaluationException;
+
+	<T> T getValue(EvaluationContext context, @Nullable Class<T> desiredResultType)
+			throws EvaluationException;
+
+	<T> T getValue(EvaluationContext context, @Nullable Object rootObject,
+			@Nullable Class<T> desiredResultType) throws EvaluationException;
+
 	Class<?> getValueType() throws EvaluationException;
 
+	Class<?> getValueType(@Nullable Object rootObject) throws EvaluationException;
+
 	Class<?> getValueType(EvaluationContext context) throws EvaluationException;
 
-	void setValue(Object rootObject, Object value) throws EvaluationException;
-}
+	Class<?> getValueType(EvaluationContext context, @Nullable Object rootObject)
+			throws EvaluationException;
+
+	boolean isWritable(@Nullable Object rootObject) throws EvaluationException;
+
+	boolean isWritable(EvaluationContext context) throws EvaluationException;
+
+	boolean isWritable(EvaluationContext context, @Nullable Object rootObject)
+			throws EvaluationException;
+
+	void setValue(@Nullable Object rootObject, @Nullable Object value) throws EvaluationException;
+
+	void setValue(EvaluationContext context, @Nullable Object value) throws EvaluationException;
+
+	void setValue(EvaluationContext context, @Nullable Object rootObject, @Nullable Object value)
+			throws EvaluationException;
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/ExpressionException.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2002-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.springframework.expression;
+
+import org.springframework.lang.Nullable;
+
+public class ExpressionException extends RuntimeException {
+	public ExpressionException(String message) {}
+
+	public ExpressionException(String message, Throwable cause) {}
+
+	public ExpressionException(@Nullable String expressionString, String message) {}
+
+	public ExpressionException(@Nullable String expressionString, int position, String message) {}
+
+	public ExpressionException(int position, String message) {}
+
+	public ExpressionException(int position, String message, Throwable cause) {}
+
+	public final String getExpressionString() {
+		return null;
+	}
+
+	public final int getPosition() {
+		return 0;
+	}
+
+	@Override
+	public String getMessage() {
+		return null;
+	}
+
+	public String toDetailedString() {
+		return null;
+	}
+
+	public String getSimpleMessage() {
+		return null;
+	}
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/ExpressionParser.java

@@ -1,6 +1,23 @@
+/*
+ * Copyright 2002-2009 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
 package org.springframework.expression;
 
 public interface ExpressionParser {
+    Expression parseExpression(String expressionString) throws ParseException;
 
-    Expression parseExpression(String string);
-}
+    Expression parseExpression(String expressionString, ParserContext context)
+            throws ParseException;
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/ParseException.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2002-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.springframework.expression;
+
+import org.springframework.lang.Nullable;
+
+public class ParseException extends ExpressionException {
+	public ParseException(@Nullable String expressionString, int position, String message) {
+		super(expressionString, message);
+	}
+
+	public ParseException(int position, String message, Throwable cause) {
+		super(message, cause);
+	}
+
+	public ParseException(int position, String message) {
+		super(message);
+	}
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/ParserContext.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2002-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.springframework.expression;
+
+public interface ParserContext {
+	boolean isTemplate();
+
+	String getExpressionPrefix();
+
+	String getExpressionSuffix();
+
+	ParserContext TEMPLATE_EXPRESSION = null;
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/common/TemplateAwareExpressionParser.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2002-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.springframework.expression.common;
+
+import org.springframework.expression.Expression;
+import org.springframework.expression.ExpressionParser;
+import org.springframework.expression.ParseException;
+import org.springframework.expression.ParserContext;
+import org.springframework.lang.Nullable;
+
+public abstract class TemplateAwareExpressionParser implements ExpressionParser {
+	@Override
+	public Expression parseExpression(String expressionString) throws ParseException {
+		return null;
+	}
+
+	@Override
+	public Expression parseExpression(String expressionString, @Nullable ParserContext context)
+			throws ParseException {
+		return null;
+	}
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/spel/standard/SpelExpression.java

@@ -0,0 +1,137 @@
+/*
+ * Copyright 2002-2020 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.springframework.expression.spel.standard;
+
+import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.EvaluationException;
+import org.springframework.expression.Expression;
+import org.springframework.lang.Nullable;
+
+public class SpelExpression implements Expression {
+	public void setEvaluationContext(EvaluationContext evaluationContext) {}
+
+	public EvaluationContext getEvaluationContext() {
+		return null;
+	}
+
+	@Override
+	public String getExpressionString() {
+		return null;
+	}
+
+	@Override
+	public Object getValue() throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public <T> T getValue(@Nullable Class<T> expectedResultType) throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Object getValue(@Nullable Object rootObject) throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public <T> T getValue(@Nullable Object rootObject, @Nullable Class<T> expectedResultType)
+			throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Object getValue(EvaluationContext context) throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public <T> T getValue(EvaluationContext context, @Nullable Class<T> expectedResultType)
+			throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Object getValue(EvaluationContext context, @Nullable Object rootObject)
+			throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public <T> T getValue(EvaluationContext context, @Nullable Object rootObject,
+			@Nullable Class<T> expectedResultType) throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Class<?> getValueType() throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Class<?> getValueType(@Nullable Object rootObject) throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Class<?> getValueType(EvaluationContext context) throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public Class<?> getValueType(EvaluationContext context, @Nullable Object rootObject)
+			throws EvaluationException {
+		return null;
+	}
+
+	@Override
+	public boolean isWritable(@Nullable Object rootObject) throws EvaluationException {
+		return false;
+	}
+
+	@Override
+	public boolean isWritable(EvaluationContext context) throws EvaluationException {
+		return false;
+	}
+
+	@Override
+	public boolean isWritable(EvaluationContext context, @Nullable Object rootObject)
+			throws EvaluationException {
+		return false;
+	}
+
+	@Override
+	public void setValue(@Nullable Object rootObject, @Nullable Object value)
+			throws EvaluationException {}
+
+	@Override
+	public void setValue(EvaluationContext context, @Nullable Object value)
+			throws EvaluationException {}
+
+	@Override
+	public void setValue(EvaluationContext context, @Nullable Object rootObject,
+			@Nullable Object value) throws EvaluationException {}
+
+	public boolean compileExpression() {
+		return false;
+	}
+
+	public void revertToInterpreted() {}
+
+	public String toStringAST() {
+		return null;
+	}
+
+}

java/ql/test/stubs/springframework-5.3.8/org/springframework/expression/spel/standard/SpelExpressionParser.java

@@ -1,10 +1,26 @@
+/*
+ * Copyright 2002-2014 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
 package org.springframework.expression.spel.standard;
 
-import org.springframework.expression.*;
-
-public class SpelExpressionParser implements ExpressionParser {
+import org.springframework.expression.ParseException;
+import org.springframework.expression.common.TemplateAwareExpressionParser;
 
+public class SpelExpressionParser extends TemplateAwareExpressionParser {
     public SpelExpressionParser() {}
-    
-    public Expression parseExpression(String string) { return null; }
-}
+
+    public SpelExpression parseRaw(String expressionString) throws ParseException {
+        return null;
+    }
+}