Merge pull request github#6001 from atorralba/atorralba/promote-mvel-injection

Java: Promote MVEL injection query from experimental

Author: aschackmull

java/change-notes/2021-06-02-mvel-injection-query.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The query "Expression language injection (MVEL) (`java/mvel-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/3329)

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

@@ -3,11 +3,11 @@
 
 <overview>
 <p>
-MVEL is an expression language based on Java-syntax. 
-The language offers many features
+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 the attacker to run arbitrary code.
+and then evaluated, then it may allow attackers to run arbitrary code.
 </p>
 </overview>
 
@@ -19,10 +19,12 @@ Including user input in a MVEL expression should be avoided.
 
 <example>
 <p>
-The following example uses untrusted data to build a MVEL expression
-and then runs it in the default powerfull context.
+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="UnsafeMvelExpressionEvaluation.java" />
+<sample src="MvelExpressionEvaluation.java" />
 </example>
 
 <references>
@@ -35,4 +37,4 @@ and then runs it in the default powerfull context.
   <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/MvelInjection.ql renamed to java/ql/src/Security/CWE/CWE-094/MvelInjection.ql

@@ -11,9 +11,9 @@
  */
 
 import java
-import MvelInjectionLib
+import semmle.code.java.security.MvelInjectionQuery
 import DataFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionConfig conf
+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"