From fc2b18ae8a7a4005c348abb15d6a70d40fd31659 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jbj@github.com>
Date: Mon, 7 Oct 2024 13:20:55 +0200
Subject: [PATCH] Java: Diff-informed CleartextStorageCookie.ql

This query shares implementation with several other queries about
cleartext storage, but it's the only one of them that's in the
code-scanning suite. The sharing mechanism remains the same as before,
but now each query has to override `getASelectedLocation` to become
diff-informed.

Two other data-flow configurations are used in this query, but they
can't easily be made diff-informed.
---
 .../security/CleartextStorageCookieQuery.qll  | 12 ++++++++++-
 .../java/security/CleartextStorageQuery.qll   | 20 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll
index 1f262ad57d61..1c99821386da 100644
--- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll
@@ -7,7 +7,17 @@ private import semmle.code.java.dataflow.FlowSinks
 private import semmle.code.java.dataflow.FlowSources
 
 private class CookieCleartextStorageSink extends CleartextStorageSink {
-  CookieCleartextStorageSink() { this.asExpr() = cookieInput(_) }
+  Cookie cookie;
+
+  CookieCleartextStorageSink() { this.asExpr() = cookieInput(cookie) }
+
+  override Location getASelectedLocation() {
+    result = this.getLocation()
+    or
+    result = cookie.getLocation()
+    or
+    result = cookie.getAStore().getLocation()
+  }
 }
 
 /** The instantiation of a cookie, which can act as storage. */
diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll
index a607fd8c8d2b..21d82bef657e 100644
--- a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll
@@ -5,7 +5,14 @@ private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.security.SensitiveActions
 
 /** A sink representing persistent storage that saves data in clear text. */
-abstract class CleartextStorageSink extends DataFlow::Node { }
+abstract class CleartextStorageSink extends DataFlow::Node {
+  /**
+   * Gets a location that will be selected in the diff-informed query where
+   * this sink is found. If this has no results for any sink, that's taken to
+   * mean the query is not diff-informed.
+   */
+  Location getASelectedLocation() { none() }
+}
 
 /** A sanitizer for flows tracking sensitive data being stored in persistent storage. */
 abstract class CleartextStorageSanitizer extends DataFlow::Node { }
@@ -46,6 +53,17 @@ private module SensitiveSourceFlowConfig implements DataFlow::ConfigSig {
   predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
     any(CleartextStorageAdditionalTaintStep c).step(n1, n2)
   }
+
+  predicate observeDiffInformedIncrementalMode() {
+    // This configuration is used by several queries. A query can opt in to
+    // diff-informed mode by implementing `getASelectedLocation` on its sinks,
+    // indicating that it has considered which sinks are selected.
+    exists(CleartextStorageSink sink | exists(sink.getASelectedLocation()))
+  }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(CleartextStorageSink).getASelectedLocation()
+  }
 }
 
 private module SensitiveSourceFlow = TaintTracking::Global<SensitiveSourceFlowConfig>;