From 76438f13b640c2284a7e6d5ae3a06122883da3bb Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Mon, 5 Jun 2023 15:55:02 -0400
Subject: [PATCH] Trust Boundary Query

---
 java/ql/lib/ext/javax.servlet.http.model.yml  |  2 +
 .../security/TrustBoundaryViolationQuery.qll  | 17 +++++
 .../CWE/CWE-501/TrustBoundaryViolation.ql     | 75 +++++++++++++++++++
 3 files changed, 94 insertions(+)
 create mode 100644 java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll
 create mode 100644 java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql

diff --git a/java/ql/lib/ext/javax.servlet.http.model.yml b/java/ql/lib/ext/javax.servlet.http.model.yml
index 6485ea22a2e..1735491f4eb 100644
--- a/java/ql/lib/ext/javax.servlet.http.model.yml
+++ b/java/ql/lib/ext/javax.servlet.http.model.yml
@@ -26,6 +26,8 @@ extensions:
       - ["javax.servlet.http", "HttpServletResponse", False, "addHeader", "", "", "Argument[0..1]", "response-splitting", "manual"]
       - ["javax.servlet.http", "HttpServletResponse", False, "sendError", "(int,String)", "", "Argument[1]", "information-leak", "manual"]
       - ["javax.servlet.http", "HttpServletResponse", False, "setHeader", "", "", "Argument[0..1]", "response-splitting", "manual"]
+      - ["javax.servlet.http", "HttpSession", True, "putValue", "", "", "Argument[0..1]", "trust-boundary", "manual"]
+      - ["javax.servlet.http", "HttpSession", True, "setAttribute", "", "", "Argument[0..1]", "trust-boundary", "manual"]
   - addsTo:
       pack: codeql/java-all
       extensible: summaryModel
diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll
new file mode 100644
index 00000000000..5af886096e0
--- /dev/null
+++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll
@@ -0,0 +1,17 @@
+/** Provides classes and predicates to reason about trust boundary violations */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.frameworks.Servlets
+
+class TrustBoundaryViolationSource extends DataFlow::Node {
+  TrustBoundaryViolationSource() {
+    this instanceof RemoteFlowSource and this.asExpr().getType() instanceof HttpServletRequest
+  }
+}
+
+class TrustBoundaryViolationSink extends DataFlow::Node {
+  TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") }
+}
diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql
new file mode 100644
index 00000000000..7a2a1610d03
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql
@@ -0,0 +1,75 @@
+/**
+ * @id java/trust-boundary-violation
+ * @name Trust boundary violation
+ * @description A user-provided value is used to set a session attribute.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision medium
+ * @tags security
+ *      external/cwe/cwe-501
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.TrustBoundaryViolationQuery
+
+/**
+ * The `setAttribute` method of the `HttpSession` interface.
+ */
+abstract class SessionSetAttributeMethod extends Method {
+  abstract int getArgumentIndex();
+}
+
+private class PlayMvcResultAddingToSessionMethod extends SessionSetAttributeMethod {
+  PlayMvcResultAddingToSessionMethod() {
+    this.getDeclaringType().hasQualifiedName("play.mvc", "Result") and
+    this.hasName("addingToSession")
+  }
+
+  override int getArgumentIndex() { result = [1, 2] }
+}
+
+private class Struts2SessionMapPutMethod extends SessionSetAttributeMethod {
+  Struts2SessionMapPutMethod() {
+    this.getDeclaringType().hasQualifiedName("org.apache.struts2.dispatcher", "SessionMap") and
+    this.hasName("put")
+  }
+
+  override int getArgumentIndex() { result = 1 }
+}
+
+private class Struts2SessionSetMethod extends SessionSetAttributeMethod {
+  Struts2SessionSetMethod() {
+    this.getDeclaringType().hasQualifiedName("org.apache.struts2.interceptor", "SessionAware") and
+    this.hasName(["setSession", "withSession"])
+  }
+
+  override int getArgumentIndex() { result = 0 }
+}
+
+module TrustBoundaryConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source instanceof RemoteFlowSource and
+    source.asExpr().(MethodAccess).getQualifier().getType() instanceof HttpServletRequest
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma, SessionSetAttributeMethod m | m = ma.getMethod() |
+      sink.asExpr() = ma.getArgument(m.getArgumentIndex())
+    )
+    or
+    sink instanceof TrustBoundaryViolationSink
+  }
+}
+
+module TrustBoundaryFlow = TaintTracking::Global<TrustBoundaryConfig>;
+
+import TrustBoundaryFlow::PathGraph
+
+from TrustBoundaryFlow::PathNode source, TrustBoundaryFlow::PathNode sink
+where TrustBoundaryFlow::flowPath(source, sink)
+select sink.getNode(), sink, source,
+  "This servlet reads data from a remote source and writes it to a $@.", sink.getNode(),
+  "session variable"