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

Java: Promote SpEL Injection query from experimental

Author: aschackmull

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/experimental/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java renamed to java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java

@@ -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/experimental/Security/CWE/CWE-094/SpelInjection.qhelp renamed to java/ql/src/Security/CWE/CWE-094/SpelInjection.qhelp

@@ -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"