Merge branch 'main' into starcke/automodel-pack

java/ql/src/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.7.3
+
+No user-facing changes.
+
 ## 0.7.2
 
 ### Minor Analysis Improvements

java/ql/src/Metrics/Summaries/GeneratedVsManualCoverageQuery.qll

@@ -45,12 +45,10 @@ private int getNumApis(string package, string apiSubset) {
 
 /** Holds if the given `callable` belongs to the specified `apiSubset`. */
 private predicate callableSubset(Callable callable, string apiSubset) {
-  apiSubset in ["topJdkApis", "allApis"] and
-  (
-    if apiSubset = "topJdkApis"
-    then exists(TopJdkApi topJdkApi | callable = topJdkApi.asCallable())
-    else apiSubset = "allApis"
-  )
+  apiSubset = "topJdkApis" and
+  callable instanceof TopJdkApi
+  or
+  apiSubset = "allApis"
 }
 
 /**

java/ql/src/Metrics/Summaries/TopJdkApis.qll

@@ -1,7 +1,6 @@
 /** Provides classes and predicates for working with Top JDK APIs. */
 
 import java
-private import semmle.code.java.dataflow.FlowSummary
 private import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
 private import semmle.code.java.dataflow.ExternalFlow
 
@@ -287,24 +286,29 @@ predicate hasApiName(Callable c, string apiName) {
 }
 
 /** A top JDK API. */
-class TopJdkApi extends SummarizedCallableBase {
+class TopJdkApi extends Callable {
   TopJdkApi() {
+    this.isSourceDeclaration() and
     exists(string apiName |
-      hasApiName(this.asCallable(), apiName) and
+      hasApiName(this, apiName) and
       topJdkApiName(apiName)
     )
   }
 
   /** Holds if this API has a manual summary model. */
-  private predicate hasManualSummary() { this.(SummarizedCallable).hasManualModel() }
+  private predicate hasManualSummary() {
+    exists(FlowSummaryImpl::Public::SummarizedCallable sc |
+      sc.asCallable() = this and sc.hasManualModel()
+    )
+  }
 
-  /** Holds if this API has a manual neutral model. */
-  private predicate hasManualNeutral() {
-    this.(FlowSummaryImpl::Public::NeutralCallable).hasManualModel()
+  /** Holds if this API has a manual neutral summary model. */
+  private predicate hasManualNeutralSummary() {
+    this.(FlowSummaryImpl::Public::NeutralSummaryCallable).hasManualModel()
   }
 
   /** Holds if this API has a manual MaD model. */
-  predicate hasManualMadModel() { this.hasManualSummary() or this.hasManualNeutral() }
+  predicate hasManualMadModel() { this.hasManualSummary() or this.hasManualNeutralSummary() }
   /*
    * Note: the following top JDK APIs are not modeled with MaD:
    * `java.lang.Runnable#run()`: specialised lambda flow

java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java

@@ -0,0 +1,8 @@
+public void doGet(HttpServletRequest request, HttpServletResponse response) {
+    String username = request.getParameter("username");
+
+    if (validator.isValidInput("HTTP parameter", username, "username", 20, false)) {
+        // GOOD: The input is sanitized before being written to the session.
+        request.getSession().setAttribute("username", username);
+    }
+}

java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp

@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+ "-//Semmle//qhelp//EN"
+ "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      A trust boundary violation occurs when a value is passed from a less trusted context to a more trusted context.
+    </p>
+    
+    <p>
+      For example, a value that is generated by a less trusted source, such as a user, may be passed to a more trusted
+      source, such as a system process. If the less trusted source is malicious, then the value may be crafted to
+      exploit the more trusted source.
+    </p>
+
+    <p>
+      Trust boundary violations are often caused by a failure to validate input. For example, if a web application
+      accepts a cookie from a user, then the application should validate the cookie before using it. If the cookie is
+      not validated, then the user may be able to craft a malicious cookie that exploits the application.
+    </p>
+  </overview>
+
+  <recommendation>
+    <p>
+      To maintain a trust boundary, validate data from less trusted sources before use.
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      In the first (bad) example, the server accepts a parameter from the user, then uses it to set the username without validation.
+    </p>
+    <sample src="TrustBoundaryVulnerable.java" />
+
+    <p>
+      In the second (good) example, the server validates the parameter from the user, then uses it to set the username.
+    </p>
+    <sample src="TrustBoundaryFixed.java" />
+
+  </example>
+
+  <references>
+    <li>
+      Wikipedia: <a href="http://en.wikipedia.org/wiki/Trust_boundary">Trust boundary</a>.
+    </li>
+  </references>
+
+</qhelp>

java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql

@@ -0,0 +1,20 @@
+/**
+ * @id java/trust-boundary-violation
+ * @name Trust boundary violation
+ * @description Modifying the HTTP session attributes based on data from an untrusted source may violate a trust boundary.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.8
+ * @precision medium
+ * @tags security
+ *      external/cwe/cwe-501
+ */
+
+import java
+import semmle.code.java.security.TrustBoundaryViolationQuery
+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 session variable."

java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java

@@ -0,0 +1,6 @@
+public void doGet(HttpServletRequest request, HttpServletResponse response) {
+    String username = request.getParameter("username");
+
+    // BAD: The input is written to the session without being sanitized.
+    request.getSession().setAttribute("username", username);
+}

java/ql/src/Telemetry/ExternalApi.qll

@@ -79,7 +79,7 @@ class ExternalApi extends Callable {
 
   /** Holds if this API is a known neutral. */
   pragma[nomagic]
-  predicate isNeutral() { this = any(FlowSummaryImpl::Public::NeutralCallable nsc).asCallable() }
+  predicate isNeutral() { this instanceof FlowSummaryImpl::Public::NeutralCallable }
 
   /**
    * Holds if this API is supported by existing CodeQL libraries, that is, it is either a

java/ql/src/change-notes/2023-07-19-xxe-new-sinks.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The queries "Resolving XML external entity in user-controlled data" (`java/xxe`) and "Resolving XML external entity in user-controlled data from local source" (`java/xxe-local`) now recognize sinks in the MDHT library.

java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+* Added the `java/trust-boundary-violation` query to detect trust boundary violations between HTTP requests and the HTTP session. Also added the `trust-boundary-violation` sink kind for sinks which may cross a trust boundary, such as calls to the `HttpSession#setAttribute` method.
+

java/ql/src/change-notes/released/0.7.3.md

@@ -0,0 +1,3 @@
+## 0.7.3
+
+No user-facing changes.

java/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.7.2
+lastReleaseVersion: 0.7.3

java/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/java-queries
-version: 0.7.3-dev
+version: 0.7.4-dev
 groups: 
   - java
   - queries