C++: Repair the 'cpp/cleartext-transmission' query in preparation for IR-based use-use dataflow.

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

@@ -16,6 +16,8 @@ import cpp
 import semmle.code.cpp.security.SensitiveExprs
 import semmle.code.cpp.security.PrivateData
 import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.dataflow.TaintTracking2
+import semmle.code.cpp.dataflow.TaintTracking3
 import semmle.code.cpp.models.interfaces.FlowSource
 import semmle.code.cpp.commons.File
 import DataFlow::PathGraph
@@ -34,21 +36,6 @@ class SourceFunction extends Function {
   }
 }
 
-/**
- * A DataFlow node corresponding to a variable or function call that
- * might contain or return a password or other sensitive information.
- */
-class SourceNode extends DataFlow::Node {
-  SourceNode() {
-    this.asExpr() = any(SourceVariable sv).getInitializer().getExpr() or
-    this.asExpr().(VariableAccess).getTarget() = any(SourceVariable sv).(GlobalOrNamespaceVariable) or
-    this.asExpr().(VariableAccess).getTarget() = any(SourceVariable v | v instanceof Field) or
-    this.asUninitialized() instanceof SourceVariable or
-    this.asParameter() instanceof SourceVariable or
-    this.asExpr().(FunctionCall).getTarget() instanceof SourceFunction
-  }
-}
-
 /**
  * A function that sends or receives data over a network.
  */
@@ -216,24 +203,94 @@ class Encrypted extends Expr {
   }
 }
 
+/**
+ * Holds if `sink` is a node that represents data transmitted through a network
+ * operation `nsr`.
+ */
+predicate isSinkSendRecv(DataFlow::Node sink, NetworkSendRecv nsr) {
+  sink.asConvertedExpr() = nsr.getDataExpr().getFullyConverted()
+}
+
+/**
+ * Holds if `sink` is a node that is encrypted by `enc`.
+ */
+predicate isSinkEncrypt(DataFlow::Node sink, Encrypted enc) {
+  sink.asConvertedExpr() = enc.getFullyConverted()
+}
+
+/**
+ * Holds if `source` represents a use of a sensitive variable, or data returned by a
+ * function returning sensitive data.
+ */
+predicate isSourceImpl(DataFlow::Node source) {
+  exists(Expr e |
+    e = source.asIndirectConvertedExpr() and
+    e.getUnconverted().(VariableAccess).getTarget() instanceof SourceVariable and
+    not e.hasConversion()
+  )
+  or
+  source.asExpr().(FunctionCall).getTarget() instanceof SourceFunction
+}
+
 /**
  * A taint flow configuration for flow from a sensitive expression to a network
- * operation or encryption operation.
+ * operation.
  */
 class FromSensitiveConfiguration extends TaintTracking::Configuration {
   FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof SourceNode }
+  override predicate isSource(DataFlow::Node source) { isSourceImpl(source) }
 
-  override predicate isSink(DataFlow::Node sink) {
-    sink.asExpr() = any(NetworkSendRecv nsr).getDataExpr()
-    or
-    sink.asExpr() instanceof Encrypted
+  override predicate isSink(DataFlow::Node sink) { isSinkSendRecv(sink, _) }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
   }
 
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    // flow through encryption functions to the return value (in case we can reach other sinks)
-    node2.asExpr().(Encrypted).(FunctionCall).getAnArgument() = node1.asExpr()
+  override predicate isSanitizerIn(DataFlow::Node node) {
+    // As any use of a sensitive variable is a potential source, we need to block flow into
+    // sources to not get path duplication.
+    this.isSource(node)
+  }
+}
+
+/**
+ * A taint flow configuration for flow from a sensitive expression to an encryption operation.
+ */
+class ToEncryptionConfiguration extends TaintTracking2::Configuration {
+  ToEncryptionConfiguration() { this = "ToEncryptionConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    any(FromSensitiveConfiguration config).hasFlow(source, _) and
+    isSourceImpl(source)
+  }
+
+  override predicate isSink(DataFlow::Node sink) { isSinkEncrypt(sink, _) }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
+  }
+
+  override predicate isSanitizerIn(DataFlow::Node node) {
+    // As any use of a sensitive variable is a potential source, we need to block flow into
+    // sources to not get path duplication.
+    this.isSource(node)
+  }
+}
+
+/**
+ * A taint flow configuration for flow from an encryption operation to a network operation.
+ */
+class FromEncryptionConfiguration extends TaintTracking3::Configuration {
+  FromEncryptionConfiguration() { this = "FromEncryptionConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    isSinkEncrypt(source, _) or
+    isSinkEncrypt(_, source.asDefiningArgument())
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    any(FromSensitiveConfiguration config).hasFlowTo(sink)
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -241,18 +298,14 @@ class FromSensitiveConfiguration extends TaintTracking::Configuration {
   }
 }
 
-from
-  FromSensitiveConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
-  NetworkSendRecv networkSendRecv, string msg
+from DataFlow::PathNode source, DataFlow::PathNode sink, NetworkSendRecv networkSendRecv, string msg
 where
   // flow from sensitive -> network data
-  config.hasFlowPath(source, sink) and
-  sink.getNode().asExpr() = networkSendRecv.getDataExpr() and
+  any(FromSensitiveConfiguration config).hasFlowPath(source, sink) and
+  isSinkSendRecv(sink.getNode(), networkSendRecv) and
   // no flow from sensitive -> evidence of encryption
-  not exists(DataFlow::Node encrypted |
-    config.hasFlow(source.getNode(), encrypted) and
-    encrypted.asExpr() instanceof Encrypted
-  ) and
+  not any(ToEncryptionConfiguration config).hasFlow(source.getNode(), _) and
+  not any(FromEncryptionConfiguration config).hasFlowTo(sink.getNode()) and
   // construct result
   if networkSendRecv instanceof NetworkSend
   then
@@ -263,4 +316,4 @@ where
     msg =
       "This operation receives into '" + sink.toString() +
         "', which may put unencrypted sensitive data into $@."
-select networkSendRecv, source, sink, msg, source, source.getNode().toString()
+select networkSendRecv, source, sink, msg, source.getNode(), source.getNode().toString()