Updated JexlInjection.ql to check for sandboxes

- Added a dataflow config to track setting a sandbox
  on JexlBuilder
- Added SandboxedJexl3.java test

Author: artem-smotrakov

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

@@ -47,8 +47,10 @@ private class JexlEvaluationSink extends DataFlow::ExprNode {
       m instanceof CallableCallMethod and ma.getQualifier() = taintFrom
       or
       m instanceof JexlEngineGetSetPropertyMethod and
-      ma.getAnArgument().getType() instanceof TypeString and
-      ma.getAnArgument() = taintFrom
+      exists(Expr arg, int index | arg = ma.getArgument(index) and index = [1, 2] |
+        arg.getType() instanceof TypeString and
+        arg = taintFrom
+      )
     )
   }
 }
@@ -67,18 +69,21 @@ private class TaintPropagatingJexlMethodCall extends MethodAccess {
     |
       m instanceof CreateJexlScriptMethod and
       taintFromExpr = this.getArgument(0) and
-      taintType instanceof TypeString
+      taintType instanceof TypeString and
+      isUnsafeEngine(this.getQualifier())
       or
       m instanceof CreateJexlCallableMethod and
       taintFromExpr = this.getQualifier()
       or
       m instanceof CreateJexlExpressionMethod and
       taintFromExpr = this.getAnArgument() and
-      taintType instanceof TypeString
+      taintType instanceof TypeString and
+      isUnsafeEngine(this.getQualifier())
       or
       m instanceof CreateJexlTemplateMethod and
       (taintType instanceof TypeString or taintType instanceof Reader) and
-      taintFromExpr = this.getArgument([0, 1])
+      taintFromExpr = this.getArgument([0, 1]) and
+      isUnsafeEngine(this.getQualifier())
     )
   }
 
@@ -91,6 +96,51 @@ private class TaintPropagatingJexlMethodCall extends MethodAccess {
   }
 }
 
+/**
+ * Holds if `expr` is one of the Jexl engines that is not configured with a sandbox.
+ */
+private predicate isUnsafeEngine(Expr expr) {
+  not exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
+}
+
+/**
+ * A configuration for a tracking sandboxed Jexl engines.
+ */
+private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
+  SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      m.getDeclaringType() instanceof JexlBuilder and
+      m.hasName(["uberspect", "sandbox"]) and
+      m.getReturnType() instanceof JexlBuilder and
+      (ma = node.asExpr() or ma.getQualifier() = node.asExpr())
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    node.asExpr().getType() instanceof JexlEngine or
+    node.asExpr().getType() instanceof JxltEngine or
+    node.asExpr().getType() instanceof UnifiedJexl
+  }
+
+  override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
+    createsJexlEngine(fromNode, toNode)
+  }
+}
+
+/**
+ * Holds if `fromNode` to `toNode` is a dataflow step that creates one of the Jexl engines.
+ */
+private predicate createsJexlEngine(DataFlow::Node fromNode, DataFlow::Node toNode) {
+  exists(MethodAccess ma, Method m | m = ma.getMethod() |
+    (m.getDeclaringType() instanceof JexlBuilder or m.getDeclaringType() instanceof JexlEngine) and
+    m.hasName(["create", "createJxltEngine"]) and
+    ma.getQualifier() = fromNode.asExpr() and
+    ma = toNode.asExpr()
+  )
+}
+
 /**
  * Holds if `fromNode` to `toNode` is a dataflow step that returns data from
  * a tainted bean by calling one of its getters.
@@ -190,6 +240,10 @@ private class JexlScript extends JexlRefType {
   JexlScript() { hasName(["Script", "JexlScript"]) }
 }
 
+private class JexlBuilder extends JexlRefType {
+  JexlBuilder() { hasName("JexlBuilder") }
+}
+
 private class JexlEngine extends JexlRefType {
   JexlEngine() { hasName("JexlEngine") }
 }

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

@@ -0,0 +1,76 @@
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.security.AccessControlException;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.function.Consumer;
+
+import org.apache.commons.jexl3.*;
+import org.apache.commons.jexl3.introspection.*;
+
+public class SandboxedJexl3 {
+
+    private static void runJexlExpressionWithSandbox(String jexlExpr) {
+        JexlSandbox sandbox = new JexlSandbox(false);
+        sandbox.white(SandboxedJexl3.class.getCanonicalName());
+        JexlEngine jexl = new JexlBuilder().sandbox(sandbox).create();
+        JexlExpression e = jexl.createExpression(jexlExpr);
+        JexlContext jc = new MapContext();
+        e.evaluate(jc);
+    }
+
+    private static void runJexlExpressionWithUberspectSandbox(String jexlExpr) {
+        JexlUberspect sandbox = new JexlUberspectSandbox();
+        JexlEngine jexl = new JexlBuilder().uberspect(sandbox).create();
+        JexlExpression e = jexl.createExpression(jexlExpr);
+        JexlContext jc = new MapContext();
+        e.evaluate(jc);
+    }
+
+    private static JexlBuilder STATIC_JEXL_BUILDER;
+
+    static {
+        JexlSandbox sandbox = new JexlSandbox(false);
+        sandbox.white(SandboxedJexl3.class.getCanonicalName());
+        STATIC_JEXL_BUILDER = new JexlBuilder().sandbox(sandbox);
+    }
+
+    private static void runJexlExpressionViaJxltEngineWithSandbox(String jexlExpr) {
+        JexlEngine jexl = STATIC_JEXL_BUILDER.create();
+        JxltEngine jxlt = jexl.createJxltEngine();
+        jxlt.createExpression(jexlExpr).evaluate(new MapContext());
+    }
+
+    private static class JexlUberspectSandbox implements JexlUberspect {
+
+    }
+
+    private static void withSocket(Consumer<String> action) throws Exception {
+        try (ServerSocket serverSocket = new ServerSocket(0)) {
+            try (Socket socket = serverSocket.accept()) {
+                byte[] bytes = new byte[1024];
+                int n = socket.getInputStream().read(bytes);
+                String jexlExpr = new String(bytes, 0, n);
+                action.accept(jexlExpr);
+            }
+        }
+    }
+
+    // below are examples of safer Jexl usage
+
+    // with JexlSandbox
+    public static void saferJexlExpressionInSandbox() throws Exception {
+        withSocket(SandboxedJexl3::runJexlExpressionWithSandbox);
+    }
+
+    // with a custom sandbox implemented with JexlUberspect
+    public static void saferJexlExpressionInUberspectSandbox() throws Exception {
+        withSocket(SandboxedJexl3::runJexlExpressionWithUberspectSandbox);
+    }
+
+    // with JexlSandbox and JxltEngine
+    public static void saferJxltExpressionInSandbox() throws Exception {
+        withSocket(SandboxedJexl3::runJexlExpressionViaJxltEngineWithSandbox);
+    }
+}

java/ql/test/stubs/apache-commons-jexl-3.1/org/apache/commons/jexl3/JexlBuilder.java

@@ -1,8 +1,18 @@
 package org.apache.commons.jexl3;
 
+import org.apache.commons.jexl3.introspection.*;
+
 public class JexlBuilder {
 
     public JexlEngine create() {
         return null;
     }
+
+    public JexlBuilder sandbox(JexlSandbox sandbox) {
+        return null;
+    }
+
+    public JexlBuilder uberspect(JexlUberspect uberspect) {
+        return null;
+    }
 }

java/ql/test/stubs/apache-commons-jexl-3.1/org/apache/commons/jexl3/JexlEngine.java

@@ -1,5 +1,7 @@
 package org.apache.commons.jexl3;
 
+import org.apache.commons.jexl3.introspection.*;
+
 public abstract class JexlEngine {
 
     public JexlExpression createExpression(JexlInfo info, String expression) {
@@ -31,4 +33,8 @@ public void setProperty(Object bean, String expr, Object value) {}
     public Object getProperty(Object bean, String expr) {
         return null;
     }
+
+    public JexlUberspect getUberspect() {
+        return null;
+    }
 }

java/ql/test/stubs/apache-commons-jexl-3.1/org/apache/commons/jexl3/introspection/JexlSandbox.java

@@ -0,0 +1,16 @@
+package org.apache.commons.jexl3.introspection;
+
+public class JexlSandbox {
+    
+    public JexlSandbox() {}
+
+    public JexlSandbox(boolean wb) {}
+
+    public JexlSandbox.Permissions white(String clazz) {
+        return null;
+    }
+
+    public static final class Permissions {
+
+    }
+}

java/ql/test/stubs/apache-commons-jexl-3.1/org/apache/commons/jexl3/introspection/JexlUberspect.java

@@ -0,0 +1,37 @@
+package org.apache.commons.jexl3.introspection;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+public interface JexlUberspect {
+    /*
+    interface PropertyResolver {}
+
+    List<PropertyResolver> getResolvers(JexlOperator op, Object obj);
+
+    void setClassLoader(ClassLoader loader);
+
+    ClassLoader getClassLoader();
+
+    int getVersion();
+
+    JexlMethod getConstructor(Object ctorHandle, Object... args);
+
+    JexlMethod getMethod(Object obj, String method, Object... args);
+
+    JexlPropertyGet getPropertyGet(Object obj, Object identifier);
+
+    JexlPropertyGet getPropertyGet(List<PropertyResolver> resolvers, Object obj, Object identifier);
+
+    JexlPropertySet getPropertySet(Object obj, Object identifier, Object arg);
+
+    JexlPropertySet getPropertySet(List<PropertyResolver> resolvers, Object obj, Object identifier, Object arg);
+
+    Iterator<?> getIterator(Object obj);
+
+    JexlArithmetic.Uberspect getArithmetic(JexlArithmetic arithmetic);
+    */
+}