Merge pull request #5894 from atorralba/atorralba/promote-ognl-injection

Java: Promote OGNL Injection query from experimental

java/ql/src/Security/CWE/CWE-917/OgnlInjection.java

@@ -0,0 +1,22 @@
+import ognl.Ognl;
+import ognl.OgnlException;
+
+public void evaluate(HttpServletRequest request, Object root) throws OgnlException {
+  String expression = request.getParameter("expression");
+
+  // BAD: User provided expression is evaluated
+  Ognl.getValue(expression, root);
+  
+  // GOOD: The name is validated and expression is evaluated in sandbox
+  System.setProperty("ognl.security.manager", ""); // Or add -Dognl.security.manager to JVM args
+  if (isValid(expression)) {
+    Ognl.getValue(expression, root);
+  } else {
+    // Reject the request
+  }
+}
+
+public void isValid(Strig expression) {
+  // Custom method to validate the expression.
+  // For instance, make sure it doesn't include unexpected code.
+}

java/ql/src/Security/CWE/CWE-917/OgnlInjection.qhelp

@@ -0,0 +1,35 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Object-Graph Navigation Language (OGNL) is an open-source Expression Language (EL) for Java.
+OGNL can create or change executable code, consequently it can introduce critical
+security flaws to any application that uses it. Evaluation of unvalidated expressions is a common
+flaw in OGNL. This exposes the properties of Java objects to modification by an attacker and
+may allow them to execute arbitrary code.</p>
+</overview>
+
+<recommendation>
+<p>The general recommendation is to avoid evaluating untrusted ONGL expressions. If user-provided OGNL
+expressions must be evaluated, do this in a sandbox and validate the expressions before evaluation.</p>
+</recommendation>
+
+<example>
+<p>In the following examples, the code accepts an OGNL expression from the user and evaluates it.
+</p>
+
+<p>In the first example, the user-provided OGNL expression is parsed and evaluated.</p>
+
+<p>The second example validates the expression and evaluates it inside a sandbox.
+You can add a sandbox by setting a system property, as shown in the example, or by adding
+<code>-Dognl.security.manager</code> to JVM arguments.</p>
+
+<sample src="OgnlInjection.java" />
+</example>
+
+<references>
+<li>Apache Commons: <a href="https://commons.apache.org/proper/commons-ognl/">Apache Commons OGNL</a>.</li>
+<li>Struts security: <a href="https://struts.apache.org/security/#proactively-protect-from-ognl-expression-injections-attacks-if-easily-applicable">Proactively protect from OGNL Expression Injections attacks</a>.</li>
+</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-917/OgnlInjection.ql

@@ -0,0 +1,20 @@
+/**
+ * @name OGNL Expression Language statement with user-controlled input
+ * @description Evaluation of OGNL Expression Language statement with user-controlled input can
+ *                lead to execution of arbitrary code.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/ognl-injection
+ * @tags security
+ *       external/cwe/cwe-917
+ */
+
+import java
+import semmle.code.java.security.OgnlInjectionQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, OgnlInjectionFlowConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "OGNL expression might include data from $@.",
+  source.getNode(), "this user input"