Merge pull request #10300 from geoffw0/cleartext-perf

C++: Multiple minor improvements to the cpp/cleartext-* queries

Author: geoffw0

cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.ql

@@ -19,7 +19,7 @@ import semmle.code.cpp.ir.dataflow.TaintTracking
 import DataFlow::PathGraph
 
 /**
- * Taint flow from user input to a buffer write.
+ * A taint flow configuration for flow from user input to a buffer write.
  */
 class ToBufferConfiguration extends TaintTracking::Configuration {
   ToBufferConfiguration() { this = "ToBufferConfiguration" }

cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql

@@ -21,14 +21,18 @@ import semmle.code.cpp.dataflow.TaintTracking
 import DataFlow::PathGraph
 
 /**
- * Taint flow from a sensitive expression to a `FileWrite` sink.
+ * A taint flow configuration for flow from a sensitive expression to a `FileWrite` sink.
  */
 class FromSensitiveConfiguration extends TaintTracking::Configuration {
   FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
 
   override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
 
   override predicate isSink(DataFlow::Node sink) { any(FileWrite w).getASource() = sink.asExpr() }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
+  }
 }
 
 /**

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -217,7 +217,8 @@ class Encrypted extends Expr {
 }
 
 /**
- * Taint flow from a sensitive expression.
+ * A taint flow configuration for flow from a sensitive expression to a network
+ * operation or encryption operation.
  */
 class FromSensitiveConfiguration extends TaintTracking::Configuration {
   FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
@@ -234,6 +235,10 @@ class FromSensitiveConfiguration extends TaintTracking::Configuration {
     // flow through encryption functions to the return value (in case we can reach other sinks)
     node2.asExpr().(Encrypted).(FunctionCall).getAnArgument() = node1.asExpr()
   }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
+  }
 }
 
 from

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql

@@ -13,14 +13,8 @@
 
 import cpp
 import semmle.code.cpp.security.SensitiveExprs
-import semmle.code.cpp.security.TaintTracking
-import TaintedWithPath
-
-class UserInputIsSensitiveExpr extends SecurityOptions {
-  override predicate isUserInput(Expr expr, string cause) {
-    expr instanceof SensitiveExpr and cause = "sensitive information"
-  }
-}
+import semmle.code.cpp.dataflow.TaintTracking
+import DataFlow::PathGraph
 
 class SqliteFunctionCall extends FunctionCall {
   SqliteFunctionCall() { this.getTarget().getName().matches("sqlite%") }
@@ -34,25 +28,51 @@ predicate sqlite_encryption_used() {
   any(FunctionCall fc).getTarget().getName().matches("sqlite%\\_key\\_%")
 }
 
-class Configuration extends TaintTrackingConfiguration {
-  override predicate isSource(Expr source) {
-    super.isSource(source) and source instanceof SensitiveExpr
+/**
+ *  Gets a field of the class `c`, or of another class contained in `c`.
+ */
+Field getRecField(Class c) {
+  result = c.getAField() or
+  result = getRecField(c.getAField().getUnspecifiedType().stripType())
+}
+
+/**
+ * A taint flow configuration for flow from a sensitive expression to a `SqliteFunctionCall` sink.
+ */
+class FromSensitiveConfiguration extends TaintTracking::Configuration {
+  FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
+
+  override predicate isSink(DataFlow::Node sink) {
+    any(SqliteFunctionCall c).getASource() = sink.asExpr() and
+    not sqlite_encryption_used()
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
   }
 
-  override predicate isSink(Element taintedArg) {
-    exists(SqliteFunctionCall sqliteCall |
-      taintedArg = sqliteCall.getASource() and
-      not sqlite_encryption_used()
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet content) {
+    // flow out from fields at the sink (only).
+    this.isSink(node) and
+    // constrain `content` to a field inside the node.
+    exists(Class c |
+      node.asExpr().getUnspecifiedType().stripType() = c and
+      content.(DataFlow::FieldContent).getField() = getRecField(c)
     )
+    or
+    // any default implicit reads
+    super.allowImplicitRead(node, content)
   }
 }
 
 from
-  SensitiveExpr taintSource, Expr taintedArg, SqliteFunctionCall sqliteCall, PathNode sourceNode,
-  PathNode sinkNode
+  FromSensitiveConfiguration config, SensitiveExpr sensitive, DataFlow::PathNode source,
+  DataFlow::PathNode sink, SqliteFunctionCall sqliteCall
 where
-  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
-  taintedArg = sqliteCall.getASource()
-select sqliteCall, sourceNode, sinkNode,
-  "This SQLite call may store $@ in a non-encrypted SQLite database", taintSource,
-  "sensitive information"
+  config.hasFlowPath(source, sink) and
+  source.getNode().asExpr() = sensitive and
+  sqliteCall.getASource() = sink.getNode().asExpr()
+select sqliteCall, source, sink, "This SQLite call may store $@ in a non-encrypted SQLite database",
+  sensitive, "sensitive information"

cpp/ql/src/change-notes/2022-09-05-cleartext-queries.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Modernizations from "Cleartext storage of sensitive information in buffer" (`cpp/cleartext-storage-buffer`) have been ported to the "Cleartext storage of sensitive information in file" (`cpp/cleartext-storage-file`), "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) and "Cleartext storage of sensitive information in an SQLite database" (`cpp/cleartext-storage-database`) queries. These changes may result in more correct results and fewer false positive results from these queries.

cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextFileWrite.expected

@@ -1,5 +1,4 @@
 edges
-| test2.cpp:52:44:52:57 | password_tries | test2.cpp:52:40:52:58 | * ... |
 | test2.cpp:62:18:62:25 | password | test2.cpp:65:31:65:34 | cpy1 |
 | test2.cpp:72:17:72:24 | password | test2.cpp:73:30:73:32 | buf |
 | test2.cpp:72:17:72:24 | password | test2.cpp:76:30:76:32 | buf |
@@ -9,8 +8,6 @@ nodes
 | test2.cpp:44:37:44:45 | thepasswd | semmle.label | thepasswd |
 | test2.cpp:45:38:45:47 | accountkey | semmle.label | accountkey |
 | test2.cpp:50:41:50:53 | passwd_config | semmle.label | passwd_config |
-| test2.cpp:52:40:52:58 | * ... | semmle.label | * ... |
-| test2.cpp:52:44:52:57 | password_tries | semmle.label | password_tries |
 | test2.cpp:54:41:54:52 | widepassword | semmle.label | widepassword |
 | test2.cpp:55:40:55:51 | widepassword | semmle.label | widepassword |
 | test2.cpp:57:39:57:49 | call to getPassword | semmle.label | call to getPassword |