Merge branch 'main' into atorralba/promote-groovy-injection

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

@@ -0,0 +1,25 @@
+public void evaluate(Socket socket) throws IOException {
+  try (BufferedReader reader = new BufferedReader(
+    new InputStreamReader(socket.getInputStream()))) {
+  
+    String expression = reader.readLine();
+    // BAD: the user-provided expression is directly evaluated
+    MVEL.eval(expression);
+  }
+}
+
+public void safeEvaluate(Socket socket) throws IOException {
+  try (BufferedReader reader = new BufferedReader(
+    new InputStreamReader(socket.getInputStream()))) {
+  
+    String expression = reader.readLine();
+    // GOOD: the user-provided expression is validated before evaluation
+    validateExpression(expression);
+    MVEL.eval(expression);
+  }
+}
+
+private void validateExpression(String expression) {
+  // Validate that the expression does not contain unexpected code.
+  // For instance, this can be done with allow-lists or deny-lists of code patterns.
+}

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

@@ -0,0 +1,40 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+MVEL is an expression language based on Java-syntax, 
+which offers many features
+including invocation of methods available in the JVM.
+If a MVEL expression is built using attacker-controlled data, 
+and then evaluated, then it may allow attackers to run arbitrary code.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Including user input in a MVEL expression should be avoided.
+</p>
+</recommendation>
+
+<example>
+<p>
+In the following sample, the first example uses untrusted data to build a MVEL expression
+and then runs it in the default context. In the second example, the untrusted data is
+validated with a custom method that checks that the expression does not contain unexpected code
+before evaluating it.
+</p>
+<sample src="MvelExpressionEvaluation.java" />
+</example>
+
+<references>
+<li>
+  MVEL Documentation:
+  <a href="http://mvel.documentnode.com/">Language Guide for 2.0</a>.
+</li>
+<li>
+  OWASP:
+  <a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
+</li>
+</references>
+</qhelp>

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

@@ -0,0 +1,19 @@
+/**
+ * @name Expression language injection (MVEL)
+ * @description Evaluation of a user-controlled MVEL expression
+ *              may lead to remote code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/mvel-expression-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import semmle.code.java.security.MvelInjectionQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionFlowConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "MVEL injection from $@.", source.getNode(), "this user input"

java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.qhelp

@@ -14,8 +14,8 @@ may have unforeseen effects, such as the execution of arbitrary code.
 </p>
 <p>
 There are many different serialization frameworks.  This query currently
-supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap 
-and Java IO serialization through <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
+supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap,
+Jackson and Java IO serialization through <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
 </p>
 </overview>
 
@@ -91,6 +91,15 @@ Remote code execution in JYaml library:
 JsonIO deserialization vulnerabilities:
 <a href="https://klezvirus.github.io/Advanced-Web-Hacking/Serialisation/">JsonIO deserialization</a>.
 </li>
+<li>
+Research by Moritz Bechler:
+<a href="https://www.github.com/mbechler/marshalsec/blob/master/marshalsec.pdf?raw=true">Java Unmarshaller Security - Turning your data into code execution</a>
+</li>
+<li>
+Blog posts by the developer of Jackson libraries:
+<a href="https://cowtowncoder.medium.com/on-jackson-cves-dont-panic-here-is-what-you-need-to-know-54cd0d6e8062">On Jackson CVEs: Don’t Panic — Here is what you need to know</a>
+<a href="https://cowtowncoder.medium.com/jackson-2-10-safe-default-typing-2d018f0ce2ba">Jackson 2.10: Safe Default Typing</a>
+</li>
 </references>
 
 </qhelp>

java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql

@@ -12,51 +12,9 @@
  */
 
 import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.UnsafeDeserialization
+import semmle.code.java.security.UnsafeDeserializationQuery
 import DataFlow::PathGraph
 
-class UnsafeDeserializationConfig extends TaintTracking::Configuration {
-  UnsafeDeserializationConfig() { this = "UnsafeDeserializationConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-    exists(ClassInstanceExpr cie |
-      cie.getArgument(0) = pred.asExpr() and
-      cie = succ.asExpr() and
-      (
-        cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader or
-        cie.getConstructor().getDeclaringType() instanceof YamlBeansReader or
-        cie.getConstructor().getDeclaringType().getASupertype*() instanceof UnsafeHessianInput or
-        cie.getConstructor().getDeclaringType() instanceof BurlapInput
-      )
-    )
-    or
-    exists(MethodAccess ma |
-      ma.getMethod() instanceof BurlapInputInitMethod and
-      ma.getArgument(0) = pred.asExpr() and
-      ma.getQualifier() = succ.asExpr()
-    )
-  }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    exists(ClassInstanceExpr cie |
-      cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
-      cie = node.asExpr() and
-      exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(cie.getArgument(1)))
-    )
-    or
-    exists(MethodAccess ma |
-      ma.getMethod() instanceof JsonIoJsonToJavaMethod and
-      ma.getArgument(0) = node.asExpr() and
-      exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1)))
-    )
-  }
-}
-
 from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeDeserializationConfig conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode().(UnsafeDeserializationSink).getMethodAccess(), source, sink,