From 8b51ee8fe8f41d2cf57090b37e8274d6d22169ac Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 14 May 2024 23:34:11 +0100
Subject: [PATCH 1/7] Use additional sensitive data heuristics in
 CleartextSources

---
 .../security/internal/CleartextSources.qll    | 106 ++++++++++--------
 1 file changed, 61 insertions(+), 45 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll b/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
index f197f763bf07..58af7585b3db 100644
--- a/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
+++ b/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
@@ -9,6 +9,7 @@ private import codeql.ruby.DataFlow
 private import codeql.ruby.TaintTracking::TaintTracking
 private import codeql.ruby.dataflow.RemoteFlowSources
 private import SensitiveDataHeuristics::HeuristicNames
+private import SensitiveDataHeuristics
 private import codeql.ruby.CFG
 private import codeql.ruby.dataflow.SSA
 
@@ -92,17 +93,17 @@ module CleartextSources {
   }
 
   /**
-   * A call that might obfuscate a password, for example through hashing.
+   * A call that might obfuscate sensitive data, for example through hashing.
    */
   private class ObfuscatorCall extends Sanitizer, DataFlow::CallNode {
     ObfuscatorCall() { nameIsNotSensitive(this.getMethodName()) }
   }
 
   /**
-   * A data flow node that does not contain a clear-text password, according to its syntactic name.
+   * A data flow node that does not contain clear-text sensitive data, according to its syntactic name.
    */
-  private class NameGuidedNonCleartextPassword extends NonCleartextPassword {
-    NameGuidedNonCleartextPassword() {
+  private class NameGuidedNonCleartextSensitive extends NonCleartextSensitive {
+    NameGuidedNonCleartextSensitive() {
       exists(string name | nameIsNotSensitive(name) |
         // accessing a non-sensitive variable
         this.asExpr().getExpr().(VariableReadAccess).getVariable().getName() = name
@@ -129,18 +130,23 @@ module CleartextSources {
   }
 
   /**
-   * A data flow node that receives flow that is not a clear-text password.
+   * A data flow node that receives flow that is not clear-text sensitive data.
    */
-  class NonCleartextPasswordFlow extends NonCleartextPassword {
-    NonCleartextPasswordFlow() {
-      any(NonCleartextPassword other).(DataFlow::LocalSourceNode).flowsTo(this)
+  class NonCleartextSensitiveFlow extends NonCleartextSensitive {
+    NonCleartextSensitiveFlow() {
+      any(NonCleartextSensitive other).(DataFlow::LocalSourceNode).flowsTo(this)
     }
   }
 
   /**
-   * A data flow node that does not contain a clear-text password.
+   * DEPRECATED: Use NonCleartextSensitiveFlow instead.
    */
-  abstract private class NonCleartextPassword extends DataFlow::Node { }
+  deprecated class NonCleartextPasswordFlow = NonCleartextSensitiveFlow;
+
+  /**
+   * A data flow node that does not contain clear-text sensitive data.
+   */
+  abstract private class NonCleartextSensitive extends DataFlow::Node { }
 
   // `writeNode` assigns pair with key `name` to `val`
   private predicate hashKeyWrite(DataFlow::CallNode writeNode, string name, DataFlow::Node val) {
@@ -153,18 +159,19 @@ module CleartextSources {
   }
 
   /**
-   * A value written to a hash entry with a key that may contain password information.
+   * A value written to a hash entry with a key that may contain sensitive information.
    */
-  private class HashKeyWritePasswordSource extends Source {
+  private class HashKeyWriteSensitiveSource extends Source {
     private string name;
     private DataFlow::ExprNode recv;
 
-    HashKeyWritePasswordSource() {
-      exists(DataFlow::CallNode writeNode |
-        name.regexpMatch(maybePassword()) and
+    HashKeyWriteSensitiveSource() {
+      exists(DataFlow::CallNode writeNode, SensitiveDataClassification classification |
+        nameIndicatesSensitiveData(name, classification) and
+        not classification = SensitiveDataClassification::id() and
         not nameIsNotSensitive(name) and
         // avoid safe values assigned to presumably unsafe names
-        not this instanceof NonCleartextPassword and
+        not this instanceof NonCleartextSensitive and
         // hash[name] = val
         hashKeyWrite(writeNode, name, this) and
         recv = writeNode.getReceiver()
@@ -177,7 +184,7 @@ module CleartextSources {
     string getName() { result = name }
 
     /**
-     * Gets the name of the hash variable that this password source is assigned
+     * Gets the name of the hash variable that this sensitive source is assigned
      * to, if applicable.
      */
     LocalVariable getVariable() {
@@ -186,17 +193,20 @@ module CleartextSources {
   }
 
   /**
-   * An entry into a hash literal that may contain a password
+   * An entry into a hash literal that may contain sensitive data
    */
-  private class HashLiteralPasswordSource extends Source {
+  private class HashLiteralSensitiveSource extends Source {
     private string name;
 
-    HashLiteralPasswordSource() {
-      exists(CfgNodes::ExprNodes::HashLiteralCfgNode lit |
-        name.regexpMatch(maybePassword()) and
+    HashLiteralSensitiveSource() {
+      exists(
+        CfgNodes::ExprNodes::HashLiteralCfgNode lit, SensitiveDataClassification classification
+      |
+        nameIndicatesSensitiveData(name, classification) and
+        not classification = SensitiveDataClassification::id() and
         not nameIsNotSensitive(name) and
         // avoid safe values assigned to presumably unsafe names
-        not this instanceof NonCleartextPassword and
+        not this instanceof NonCleartextSensitive and
         // hash = { name: val }
         exists(CfgNodes::ExprNodes::PairCfgNode p | p = lit.getAKeyValuePair() |
           p.getKey().getConstantValue().getStringlikeValue() = name and
@@ -208,36 +218,42 @@ module CleartextSources {
     override string describe() { result = "a write to " + name }
   }
 
-  /** An assignment that may assign a password to a variable */
-  private class AssignPasswordVariableSource extends Source {
+  /** An assignment that may assign sensitive data to a variable */
+  private class AssignSensitiveVariableSource extends Source {
     string name;
 
-    AssignPasswordVariableSource() {
-      // avoid safe values assigned to presumably unsafe names
-      not this instanceof NonCleartextPassword and
-      name.regexpMatch(maybePassword()) and
-      not nameIsNotSensitive(name) and
-      exists(Assignment a |
-        this.asExpr().getExpr() = a.getRightOperand() and
-        a.getLeftOperand().getAVariable().getName() = name
+    AssignSensitiveVariableSource() {
+      exists(SensitiveDataClassification classification |
+        // avoid safe values assigned to presumably unsafe names
+        not this instanceof NonCleartextSensitive and
+        nameIndicatesSensitiveData(name, classification) and
+        not classification = SensitiveDataClassification::id() and
+        not nameIsNotSensitive(name) and
+        exists(Assignment a |
+          this.asExpr().getExpr() = a.getRightOperand() and
+          a.getLeftOperand().getAVariable().getName() = name
+        )
       )
     }
 
     override string describe() { result = "an assignment to " + name }
   }
 
-  /** A parameter that may contain a password. */
-  private class ParameterPasswordSource extends Source {
+  /** A parameter that may contain sensitive data. */
+  private class ParameterSensitiveSource extends Source {
     private string name;
 
-    ParameterPasswordSource() {
-      name.regexpMatch(maybePassword()) and
-      not nameIsNotSensitive(name) and
-      not this instanceof NonCleartextPassword and
-      exists(Parameter p, LocalVariable v |
-        v = p.getAVariable() and
-        v.getName() = name and
-        this.asExpr().getExpr() = v.getAnAccess()
+    ParameterSensitiveSource() {
+      exists(SensitiveDataClassification classification |
+        nameIndicatesSensitiveData(name, classification) and
+        not classification = SensitiveDataClassification::id() and
+        not nameIsNotSensitive(name) and
+        not this instanceof NonCleartextSensitive and
+        exists(Parameter p, LocalVariable v |
+          v = p.getAVariable() and
+          v.getName() = name and
+          this.asExpr().getExpr() = v.getAnAccess()
+        )
       )
     }
 
@@ -260,10 +276,10 @@ module CleartextSources {
   deprecated predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
     exists(string name, ElementReference ref, LocalVariable hashVar |
       // from `hsh[password] = "changeme"` to a `hsh[password]` read
-      nodeFrom.(HashKeyWritePasswordSource).getName() = name and
+      nodeFrom.(HashKeyWriteSensitiveSource).getName() = name and
       nodeTo.asExpr().getExpr() = ref and
       ref.getArgument(0).getConstantValue().getStringlikeValue() = name and
-      nodeFrom.(HashKeyWritePasswordSource).getVariable() = hashVar and
+      nodeFrom.(HashKeyWriteSensitiveSource).getVariable() = hashVar and
       ref.getReceiver().(VariableReadAccess).getVariable() = hashVar and
       nodeFrom.asExpr().getASuccessor*() = nodeTo.asExpr()
     )

From b0c03f6d68a923527588763ad3f9ee6f0c4e4ad9 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 15 May 2024 14:36:38 +0100
Subject: [PATCH 2/7] Allow implicit read steps on sinks

---
 ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll | 5 +++++
 ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll b/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll
index 3556401887eb..be19b9d938b7 100644
--- a/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll
+++ b/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll
@@ -44,6 +44,11 @@ private module Config implements DataFlow::ConfigSig {
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
     CL::isAdditionalTaintStep(nodeFrom, nodeTo)
   }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
+    exists(cs) and
+    isSink(node)
+  }
 }
 
 /**
diff --git a/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll b/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll
index 141b54c8c3af..b1ad369d58f0 100644
--- a/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll
+++ b/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll
@@ -43,6 +43,11 @@ private module Config implements DataFlow::ConfigSig {
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
     CS::isAdditionalTaintStep(nodeFrom, nodeTo)
   }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
+    exists(cs) and
+    isSink(node)
+  }
 }
 
 /**

From 07f03be8ccc7899caa9d2854a1ca5a1fda6cbe85 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 15 May 2024 21:38:24 +0100
Subject: [PATCH 3/7] Add unit tests

---
 .../cwe-312/CleartextStorage.expected         | 51 +++++++++++++++++++
 .../app/controllers/users_controller.rb       | 16 ++++++
 2 files changed, 67 insertions(+)

diff --git a/ruby/ql/test/query-tests/security/cwe-312/CleartextStorage.expected b/ruby/ql/test/query-tests/security/cwe-312/CleartextStorage.expected
index bad0c52a26d7..3bfe42e97d04 100644
--- a/ruby/ql/test/query-tests/security/cwe-312/CleartextStorage.expected
+++ b/ruby/ql/test/query-tests/security/cwe-312/CleartextStorage.expected
@@ -23,6 +23,29 @@ edges
 | app/controllers/users_controller.rb:58:5:58:16 | new_password | app/controllers/users_controller.rb:61:25:61:53 | "password: #{...}\\n" | provenance | AdditionalTaintStep |
 | app/controllers/users_controller.rb:58:5:58:16 | new_password | app/controllers/users_controller.rb:64:35:64:61 | "password: #{...}" | provenance | AdditionalTaintStep |
 | app/controllers/users_controller.rb:58:20:58:53 | "0157af7c38cbdd24f1616de4e5321861" | app/controllers/users_controller.rb:58:5:58:16 | new_password | provenance |  |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :SSN] | app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :SSN] | provenance |  |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :credit_card_number] | app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :credit_card_number] | provenance |  |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :password] | app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :password] | provenance |  |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 1, element :password] | app/controllers/users_controller.rb:85:5:85:8 | info [element 1, element :password] | provenance |  |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :SSN] | app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :SSN] | provenance |  |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :credit_card_number] | app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :credit_card_number] | provenance |  |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :password] | app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :password] | provenance |  |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 1, element :password] | app/controllers/users_controller.rb:76:5:76:8 | info [element 1, element :password] | provenance |  |
+| app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :SSN] | app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :SSN] | provenance |  |
+| app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :credit_card_number] | app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :credit_card_number] | provenance |  |
+| app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :password] | app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :password] | provenance |  |
+| app/controllers/users_controller.rb:79:19:79:30 | "aaaaaaaaaa" | app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :password] | provenance |  |
+| app/controllers/users_controller.rb:80:29:80:49 | "0000-0000-0000-0000" | app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :credit_card_number] | provenance |  |
+| app/controllers/users_controller.rb:81:14:81:27 | "000-00-00000" | app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :SSN] | provenance |  |
+| app/controllers/users_controller.rb:83:7:83:39 | call to [] [element :password] | app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 1, element :password] | provenance |  |
+| app/controllers/users_controller.rb:83:30:83:38 | "bbbbbbb" | app/controllers/users_controller.rb:83:7:83:39 | call to [] [element :password] | provenance |  |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :SSN] | app/controllers/users_controller.rb:85:19:85:21 | inf [element :SSN] | provenance |  |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :credit_card_number] | app/controllers/users_controller.rb:85:19:85:21 | inf [element :credit_card_number] | provenance |  |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :password] | app/controllers/users_controller.rb:85:19:85:21 | inf [element :password] | provenance |  |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 1, element :password] | app/controllers/users_controller.rb:85:19:85:21 | inf [element :password] | provenance |  |
+| app/controllers/users_controller.rb:85:19:85:21 | inf [element :SSN] | app/controllers/users_controller.rb:87:20:87:22 | inf | provenance |  |
+| app/controllers/users_controller.rb:85:19:85:21 | inf [element :credit_card_number] | app/controllers/users_controller.rb:87:20:87:22 | inf | provenance |  |
+| app/controllers/users_controller.rb:85:19:85:21 | inf [element :password] | app/controllers/users_controller.rb:87:20:87:22 | inf | provenance |  |
 | app/models/user.rb:3:5:3:16 | new_password | app/models/user.rb:5:27:5:38 | new_password | provenance |  |
 | app/models/user.rb:3:20:3:53 | "06c38c6a8a9c11a9d3b209a3193047b4" | app/models/user.rb:3:5:3:16 | new_password | provenance |  |
 | app/models/user.rb:9:5:9:16 | new_password | app/models/user.rb:11:22:11:33 | new_password | provenance |  |
@@ -59,6 +82,30 @@ nodes
 | app/controllers/users_controller.rb:58:20:58:53 | "0157af7c38cbdd24f1616de4e5321861" | semmle.label | "0157af7c38cbdd24f1616de4e5321861" |
 | app/controllers/users_controller.rb:61:25:61:53 | "password: #{...}\\n" | semmle.label | "password: #{...}\\n" |
 | app/controllers/users_controller.rb:64:35:64:61 | "password: #{...}" | semmle.label | "password: #{...}" |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :SSN] | semmle.label | info [element 0, element :SSN] |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :credit_card_number] | semmle.label | info [element 0, element :credit_card_number] |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 0, element :password] | semmle.label | info [element 0, element :password] |
+| app/controllers/users_controller.rb:76:5:76:8 | info [element 1, element :password] | semmle.label | info [element 1, element :password] |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :SSN] | semmle.label | call to [] [element 0, element :SSN] |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :credit_card_number] | semmle.label | call to [] [element 0, element :credit_card_number] |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 0, element :password] | semmle.label | call to [] [element 0, element :password] |
+| app/controllers/users_controller.rb:76:12:84:5 | call to [] [element 1, element :password] | semmle.label | call to [] [element 1, element :password] |
+| app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :SSN] | semmle.label | call to [] [element :SSN] |
+| app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :credit_card_number] | semmle.label | call to [] [element :credit_card_number] |
+| app/controllers/users_controller.rb:77:7:82:7 | call to [] [element :password] | semmle.label | call to [] [element :password] |
+| app/controllers/users_controller.rb:79:19:79:30 | "aaaaaaaaaa" | semmle.label | "aaaaaaaaaa" |
+| app/controllers/users_controller.rb:80:29:80:49 | "0000-0000-0000-0000" | semmle.label | "0000-0000-0000-0000" |
+| app/controllers/users_controller.rb:81:14:81:27 | "000-00-00000" | semmle.label | "000-00-00000" |
+| app/controllers/users_controller.rb:83:7:83:39 | call to [] [element :password] | semmle.label | call to [] [element :password] |
+| app/controllers/users_controller.rb:83:30:83:38 | "bbbbbbb" | semmle.label | "bbbbbbb" |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :SSN] | semmle.label | info [element 0, element :SSN] |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :credit_card_number] | semmle.label | info [element 0, element :credit_card_number] |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 0, element :password] | semmle.label | info [element 0, element :password] |
+| app/controllers/users_controller.rb:85:5:85:8 | info [element 1, element :password] | semmle.label | info [element 1, element :password] |
+| app/controllers/users_controller.rb:85:19:85:21 | inf [element :SSN] | semmle.label | inf [element :SSN] |
+| app/controllers/users_controller.rb:85:19:85:21 | inf [element :credit_card_number] | semmle.label | inf [element :credit_card_number] |
+| app/controllers/users_controller.rb:85:19:85:21 | inf [element :password] | semmle.label | inf [element :password] |
+| app/controllers/users_controller.rb:87:20:87:22 | inf | semmle.label | inf |
 | app/models/user.rb:3:5:3:16 | new_password | semmle.label | new_password |
 | app/models/user.rb:3:20:3:53 | "06c38c6a8a9c11a9d3b209a3193047b4" | semmle.label | "06c38c6a8a9c11a9d3b209a3193047b4" |
 | app/models/user.rb:5:27:5:38 | new_password | semmle.label | new_password |
@@ -91,6 +138,10 @@ subpaths
 | app/controllers/users_controller.rb:44:21:44:32 | new_password | app/controllers/users_controller.rb:42:20:42:53 | "78ffbec583b546bd073efd898f833184" | app/controllers/users_controller.rb:44:21:44:32 | new_password | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:42:20:42:53 | "78ffbec583b546bd073efd898f833184" | an assignment to new_password |
 | app/controllers/users_controller.rb:61:25:61:53 | "password: #{...}\\n" | app/controllers/users_controller.rb:58:20:58:53 | "0157af7c38cbdd24f1616de4e5321861" | app/controllers/users_controller.rb:61:25:61:53 | "password: #{...}\\n" | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:58:20:58:53 | "0157af7c38cbdd24f1616de4e5321861" | an assignment to new_password |
 | app/controllers/users_controller.rb:64:35:64:61 | "password: #{...}" | app/controllers/users_controller.rb:58:20:58:53 | "0157af7c38cbdd24f1616de4e5321861" | app/controllers/users_controller.rb:64:35:64:61 | "password: #{...}" | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:58:20:58:53 | "0157af7c38cbdd24f1616de4e5321861" | an assignment to new_password |
+| app/controllers/users_controller.rb:87:20:87:22 | inf | app/controllers/users_controller.rb:79:19:79:30 | "aaaaaaaaaa" | app/controllers/users_controller.rb:87:20:87:22 | inf | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:79:19:79:30 | "aaaaaaaaaa" | a write to password |
+| app/controllers/users_controller.rb:87:20:87:22 | inf | app/controllers/users_controller.rb:80:29:80:49 | "0000-0000-0000-0000" | app/controllers/users_controller.rb:87:20:87:22 | inf | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:80:29:80:49 | "0000-0000-0000-0000" | a write to credit_card_number |
+| app/controllers/users_controller.rb:87:20:87:22 | inf | app/controllers/users_controller.rb:81:14:81:27 | "000-00-00000" | app/controllers/users_controller.rb:87:20:87:22 | inf | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:81:14:81:27 | "000-00-00000" | a write to SSN |
+| app/controllers/users_controller.rb:87:20:87:22 | inf | app/controllers/users_controller.rb:83:30:83:38 | "bbbbbbb" | app/controllers/users_controller.rb:87:20:87:22 | inf | This stores sensitive data returned by $@ as clear text. | app/controllers/users_controller.rb:83:30:83:38 | "bbbbbbb" | a write to password |
 | app/models/user.rb:5:27:5:38 | new_password | app/models/user.rb:3:20:3:53 | "06c38c6a8a9c11a9d3b209a3193047b4" | app/models/user.rb:5:27:5:38 | new_password | This stores sensitive data returned by $@ as clear text. | app/models/user.rb:3:20:3:53 | "06c38c6a8a9c11a9d3b209a3193047b4" | an assignment to new_password |
 | app/models/user.rb:11:22:11:33 | new_password | app/models/user.rb:9:20:9:53 | "52652fb5c709fb6b9b5a0194af7c6067" | app/models/user.rb:11:22:11:33 | new_password | This stores sensitive data returned by $@ as clear text. | app/models/user.rb:9:20:9:53 | "52652fb5c709fb6b9b5a0194af7c6067" | an assignment to new_password |
 | app/models/user.rb:17:21:17:32 | new_password | app/models/user.rb:15:20:15:53 | "f982bf2531c149a8a1444a951b12e830" | app/models/user.rb:17:21:17:32 | new_password | This stores sensitive data returned by $@ as clear text. | app/models/user.rb:15:20:15:53 | "f982bf2531c149a8a1444a951b12e830" | an assignment to new_password |
diff --git a/ruby/ql/test/query-tests/security/cwe-312/app/controllers/users_controller.rb b/ruby/ql/test/query-tests/security/cwe-312/app/controllers/users_controller.rb
index a75f5275a8fe..806b51096659 100644
--- a/ruby/ql/test/query-tests/security/cwe-312/app/controllers/users_controller.rb
+++ b/ruby/ql/test/query-tests/security/cwe-312/app/controllers/users_controller.rb
@@ -71,4 +71,20 @@ def randomPasswordAssign
     user.password = random_password
     user.save
   end
+
+  def test 
+    info = [
+      {
+        name: "U1", 
+        password: "aaaaaaaaaa",
+        credit_card_number: "0000-0000-0000-0000",
+        SSN: "000-00-00000"
+      }, 
+      {name: "U2", password: "bbbbbbb"}
+    ]
+    info.each do |inf|
+      # BAD: Plaintext password, SSN, and CCN stored to database.
+      User.create!(inf)
+    end
+  end
 end

From 5f08371f199cdd5b2e183cea1592cb32ba7ddc6e Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 15 May 2024 21:47:27 +0100
Subject: [PATCH 4/7] Add change note

---
 ruby/ql/lib/change-notes/2024-05-15-cleartext-sources.md | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 ruby/ql/lib/change-notes/2024-05-15-cleartext-sources.md

diff --git a/ruby/ql/lib/change-notes/2024-05-15-cleartext-sources.md b/ruby/ql/lib/change-notes/2024-05-15-cleartext-sources.md
new file mode 100644
index 000000000000..2718cb773099
--- /dev/null
+++ b/ruby/ql/lib/change-notes/2024-05-15-cleartext-sources.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `CleartextSources.qll` library, used by `rb/clear-text-logging-sensitive-data` and `rb/clear-text-logging-sensitive-data`, has been updated to consider heuristics for additional categories of sensitive data. 
\ No newline at end of file

From 605fe54a06b7b038245b9c0aeb54a3262f2eff31 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 23 May 2024 14:43:25 +0200
Subject: [PATCH 5/7] Ruby: Remove two Cartesian products

---
 .../dataflow/internal/DataFlowPrivate.qll     |  3 +-
 .../ruby/dataflow/internal/DataFlowPublic.qll | 28 ++++++++++++++-----
 .../ruby/security/CleartextLoggingQuery.qll   |  2 +-
 .../ruby/security/CleartextStorageQuery.qll   |  2 +-
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
index edab51da8582..a07de3530de0 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -725,6 +725,7 @@ private module Cached {
   newtype TOptionalContentSet =
     TSingletonContent(Content c) or
     TAnyElementContent() or
+    TAnyContent() or
     TKnownOrUnknownElementContent(Content::KnownElementContent c) or
     TElementLowerBoundContent(int lower, boolean includeUnknown) {
       FlowSummaryImpl::ParsePositions::isParsedElementLowerBoundPosition(_, includeUnknown, lower)
@@ -736,7 +737,7 @@ private module Cached {
 
   cached
   class TContentSet =
-    TSingletonContent or TAnyElementContent or TKnownOrUnknownElementContent or
+    TSingletonContent or TAnyElementContent or TAnyContent or TKnownOrUnknownElementContent or
         TElementLowerBoundContent or TElementContentOfTypeContent;
 
   private predicate trackKnownValue(ConstantValue cv) {
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
index 7443f24f0389..d4e07e0653e1 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
@@ -689,6 +689,9 @@ class ContentSet extends TContentSet {
   /** Holds if this content set represents all `ElementContent`s. */
   predicate isAnyElement() { this = TAnyElementContent() }
 
+  /** Holds if this content set represents all contents. */
+  predicate isAny() { this = TAnyContent() }
+
   /**
    * Holds if this content set represents a specific known element index, or an
    * unknown element index.
@@ -737,6 +740,9 @@ class ContentSet extends TContentSet {
     this.isAnyElement() and
     result = "any element"
     or
+    this.isAny() and
+    result = "any"
+    or
     exists(Content::KnownElementContent c |
       this.isKnownOrUnknownElement(c) and
       result = c + " or unknown"
@@ -790,13 +796,8 @@ class ContentSet extends TContentSet {
     result = TUnknownElementContent()
   }
 
-  /** Gets a content that may be read from when reading from this set. */
-  Content getAReadContent() {
-    this.isSingleton(result)
-    or
-    this.isAnyElement() and
-    result instanceof Content::ElementContent
-    or
+  pragma[nomagic]
+  private Content getAnElementReadContent() {
     exists(Content::KnownElementContent c | this.isKnownOrUnknownElement(c) |
       result = c or
       result = TSplatContent(c.getIndex().getInt(), _) or
@@ -832,6 +833,19 @@ class ContentSet extends TContentSet {
       result = TUnknownElementContent()
     )
   }
+
+  /** Gets a content that may be read from when reading from this set. */
+  Content getAReadContent() {
+    this.isSingleton(result)
+    or
+    this.isAnyElement() and
+    result instanceof Content::ElementContent
+    or
+    this.isAny() and
+    exists(result)
+    or
+    result = this.getAnElementReadContent()
+  }
 }
 
 /**
diff --git a/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll b/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll
index be19b9d938b7..2f321939ec20 100644
--- a/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll
+++ b/ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll
@@ -46,7 +46,7 @@ private module Config implements DataFlow::ConfigSig {
   }
 
   predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
-    exists(cs) and
+    cs.isAny() and
     isSink(node)
   }
 }
diff --git a/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll b/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll
index b1ad369d58f0..2a1a45bfb0b9 100644
--- a/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll
+++ b/ruby/ql/lib/codeql/ruby/security/CleartextStorageQuery.qll
@@ -45,7 +45,7 @@ private module Config implements DataFlow::ConfigSig {
   }
 
   predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
-    exists(cs) and
+    cs.isAny() and
     isSink(node)
   }
 }

From 90d6f2ece32a46b0514056a3ecbae0a1c08b2ec1 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 28 May 2024 09:59:00 +0100
Subject: [PATCH 6/7] Factor out nameIndicatesRelevantSensitiveData

---
 .../security/internal/CleartextSources.qll    | 58 ++++++++++---------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll b/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
index 58af7585b3db..c298e9306c21 100644
--- a/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
+++ b/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
@@ -40,6 +40,18 @@ module CleartextSources {
     re.getConstantValue().getStringlikeValue() = [".*", ".+"]
   }
 
+  /** Holds if the given name indicates the presence of sensitive data that is relevant to consider for Cleartext Storage queries. */
+  bindingset[name]
+  private predicate nameIndicatesRelevantSensitiveData(string name) {
+    exists(SensitiveDataClassification classification |
+      nameIndicatesSensitiveData(name, classification) and
+      classification in [
+          SensitiveDataClassification::password(), SensitiveDataClassification::certificate(),
+          SensitiveDataClassification::secret(), SensitiveDataClassification::private(),
+        ]
+    )
+  }
+
   /**
    * Holds if `re` may be a regular expression that can be used to sanitize
    * sensitive data with a call to `gsub`.
@@ -166,9 +178,8 @@ module CleartextSources {
     private DataFlow::ExprNode recv;
 
     HashKeyWriteSensitiveSource() {
-      exists(DataFlow::CallNode writeNode, SensitiveDataClassification classification |
-        nameIndicatesSensitiveData(name, classification) and
-        not classification = SensitiveDataClassification::id() and
+      exists(DataFlow::CallNode writeNode |
+        nameIndicatesRelevantSensitiveData(name) and
         not nameIsNotSensitive(name) and
         // avoid safe values assigned to presumably unsafe names
         not this instanceof NonCleartextSensitive and
@@ -199,11 +210,8 @@ module CleartextSources {
     private string name;
 
     HashLiteralSensitiveSource() {
-      exists(
-        CfgNodes::ExprNodes::HashLiteralCfgNode lit, SensitiveDataClassification classification
-      |
-        nameIndicatesSensitiveData(name, classification) and
-        not classification = SensitiveDataClassification::id() and
+      exists(CfgNodes::ExprNodes::HashLiteralCfgNode lit |
+        nameIndicatesRelevantSensitiveData(name) and
         not nameIsNotSensitive(name) and
         // avoid safe values assigned to presumably unsafe names
         not this instanceof NonCleartextSensitive and
@@ -223,16 +231,13 @@ module CleartextSources {
     string name;
 
     AssignSensitiveVariableSource() {
-      exists(SensitiveDataClassification classification |
-        // avoid safe values assigned to presumably unsafe names
-        not this instanceof NonCleartextSensitive and
-        nameIndicatesSensitiveData(name, classification) and
-        not classification = SensitiveDataClassification::id() and
-        not nameIsNotSensitive(name) and
-        exists(Assignment a |
-          this.asExpr().getExpr() = a.getRightOperand() and
-          a.getLeftOperand().getAVariable().getName() = name
-        )
+      // avoid safe values assigned to presumably unsafe names
+      not this instanceof NonCleartextSensitive and
+      nameIndicatesRelevantSensitiveData(name) and
+      not nameIsNotSensitive(name) and
+      exists(Assignment a |
+        this.asExpr().getExpr() = a.getRightOperand() and
+        a.getLeftOperand().getAVariable().getName() = name
       )
     }
 
@@ -244,16 +249,13 @@ module CleartextSources {
     private string name;
 
     ParameterSensitiveSource() {
-      exists(SensitiveDataClassification classification |
-        nameIndicatesSensitiveData(name, classification) and
-        not classification = SensitiveDataClassification::id() and
-        not nameIsNotSensitive(name) and
-        not this instanceof NonCleartextSensitive and
-        exists(Parameter p, LocalVariable v |
-          v = p.getAVariable() and
-          v.getName() = name and
-          this.asExpr().getExpr() = v.getAnAccess()
-        )
+      nameIndicatesRelevantSensitiveData(name) and
+      not nameIsNotSensitive(name) and
+      not this instanceof NonCleartextSensitive and
+      exists(Parameter p, LocalVariable v |
+        v = p.getAVariable() and
+        v.getName() = name and
+        this.asExpr().getExpr() = v.getAnAccess()
       )
     }
 

From eee7f5a896e77d6efef34e09d188560a357b88e2 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 17 Jun 2024 15:00:03 +0100
Subject: [PATCH 7/7] Use a combined regex for performance

---
 .../security/internal/CleartextSources.qll    | 30 ++++++++++++++-----
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll b/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
index c298e9306c21..dc31b7f49ee1 100644
--- a/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
+++ b/ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll
@@ -40,16 +40,32 @@ module CleartextSources {
     re.getConstantValue().getStringlikeValue() = [".*", ".+"]
   }
 
+  /** Holds if `c` is a sensitive data classification that is relevant to consider for Cleartext Storage queries. */
+  private predicate isRelevantClassification(SensitiveDataClassification c) {
+    c =
+      [
+        SensitiveDataClassification::password(), SensitiveDataClassification::certificate(),
+        SensitiveDataClassification::secret(), SensitiveDataClassification::private()
+      ]
+  }
+
+  pragma[noinline]
+  private string getCombinedRelevantSensitiveRegexp() {
+    // Combine all the maybe-sensitive regexps into one using non-capturing groups and |.
+    result =
+      "(?:" +
+        strictconcat(string r, SensitiveDataClassification c |
+          r = maybeSensitiveRegexp(c) and isRelevantClassification(c)
+        |
+          r, ")|(?:"
+        ) + ")"
+  }
+
   /** Holds if the given name indicates the presence of sensitive data that is relevant to consider for Cleartext Storage queries. */
   bindingset[name]
   private predicate nameIndicatesRelevantSensitiveData(string name) {
-    exists(SensitiveDataClassification classification |
-      nameIndicatesSensitiveData(name, classification) and
-      classification in [
-          SensitiveDataClassification::password(), SensitiveDataClassification::certificate(),
-          SensitiveDataClassification::secret(), SensitiveDataClassification::private(),
-        ]
-    )
+    name.regexpMatch(getCombinedRelevantSensitiveRegexp()) and
+    not name.regexpMatch(notSensitiveRegexp())
   }
 
   /**