Merge pull request github#12443 from jketema/even-more-configsig

C++: Update more queries with `DataFlow::ConfigSig`

Author: jketema

cpp/ql/lib/change-notes/2023-03-08-deprecated-dataflow-configurations.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The `WriteConfig` taint tracking configuration has been deprecated. Please use `WriteFlow`.

cpp/ql/lib/experimental/semmle/code/cpp/security/PrivateCleartextWrite.qll

@@ -36,7 +36,7 @@ module PrivateCleartextWrite {
     }
   }
 
-  class WriteConfig extends TaintTracking::Configuration {
+  deprecated class WriteConfig extends TaintTracking::Configuration {
     WriteConfig() { this = "Write configuration" }
 
     override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -46,6 +46,16 @@ module PrivateCleartextWrite {
     override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
   }
 
+  private module WriteConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+  }
+
+  module WriteFlow = TaintTracking::Make<WriteConfig>;
+
   class PrivateDataSource extends Source {
     PrivateDataSource() { this.getExpr() instanceof PrivateDataExpr }
   }

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

@@ -120,7 +120,7 @@ predicate isNonConst(DataFlow::Node node, boolean isIndirect) {
 }
 
 pragma[noinline]
-predicate isSanitizerNode(DataFlow::Node node) {
+predicate isBarrierNode(DataFlow::Node node) {
   underscoreMacro([node.asExpr(), node.asIndirectExpr()])
   or
   exists(node.asExpr()) and
@@ -132,27 +132,27 @@ predicate isSinkImpl(DataFlow::Node sink, Expr formatString) {
   exists(FormattingFunctionCall fc | formatString = fc.getArgument(fc.getFormatParameterIndex()))
 }
 
-class NonConstFlow extends TaintTracking::Configuration {
-  NonConstFlow() { this = "NonConstFlow" }
-
-  override predicate isSource(DataFlow::Node source) {
+module NonConstFlowConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     exists(boolean isIndirect, Type t |
       isNonConst(source, isIndirect) and
       t = source.getType() and
       not cannotContainString(t, isIndirect)
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) { isSinkImpl(sink, _) }
+  predicate isSink(DataFlow::Node sink) { isSinkImpl(sink, _) }
 
-  override predicate isSanitizer(DataFlow::Node node) { isSanitizerNode(node) }
+  predicate isBarrier(DataFlow::Node node) { isBarrierNode(node) }
 }
 
+module NonConstFlow = TaintTracking::Make<NonConstFlowConfiguration>;
+
 from FormattingFunctionCall call, Expr formatString
 where
   call.getArgument(call.getFormatParameterIndex()) = formatString and
-  exists(NonConstFlow cf, DataFlow::Node sink |
-    cf.hasFlowTo(sink) and
+  exists(DataFlow::Node sink |
+    NonConstFlow::hasFlowTo(sink) and
     isSinkImpl(sink, formatString)
   )
 select formatString,

cpp/ql/src/Likely Bugs/Leap Year/Adding365DaysPerYear.ql

@@ -14,8 +14,10 @@
 import cpp
 import LeapYear
 
-from Expr source, Expr sink, PossibleYearArithmeticOperationCheckConfiguration config
-where config.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(sink))
+from Expr source, Expr sink
+where
+  PossibleYearArithmeticOperationCheckFlow::hasFlow(DataFlow::exprNode(source),
+    DataFlow::exprNode(sink))
 select sink,
   "An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios.",
   source, source.toString()

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll

@@ -206,10 +206,10 @@ class ChecksForLeapYearFunctionCall extends FunctionCall {
 }
 
 /**
- * `DataFlow::Configuration` for finding a variable access that would flow into
+ * Data flow configuration for finding a variable access that would flow into
  * a function call that includes an operation to check for leap year.
  */
-class LeapYearCheckConfiguration extends DataFlow::Configuration {
+deprecated class LeapYearCheckConfiguration extends DataFlow::Configuration {
   LeapYearCheckConfiguration() { this = "LeapYearCheckConfiguration" }
 
   override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof VariableAccess }
@@ -220,9 +220,24 @@ class LeapYearCheckConfiguration extends DataFlow::Configuration {
 }
 
 /**
- * `DataFlow::Configuration` for finding an operation with hardcoded 365 that will flow into a `FILEINFO` field.
+ * Data flow configuration for finding a variable access that would flow into
+ * a function call that includes an operation to check for leap year.
+ */
+private module LeapYearCheckConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof VariableAccess }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(ChecksForLeapYearFunctionCall fc | sink.asExpr() = fc.getAnArgument())
+  }
+}
+
+module LeapYearCheckFlow = DataFlow::Make<LeapYearCheckConfiguration>;
+
+/**
+ * Data flow configuration for finding an operation with hardcoded 365 that will flow into
+ * a `FILEINFO` field.
  */
-class FiletimeYearArithmeticOperationCheckConfiguration extends DataFlow::Configuration {
+deprecated class FiletimeYearArithmeticOperationCheckConfiguration extends DataFlow::Configuration {
   FiletimeYearArithmeticOperationCheckConfiguration() {
     this = "FiletimeYearArithmeticOperationCheckConfiguration"
   }
@@ -245,10 +260,36 @@ class FiletimeYearArithmeticOperationCheckConfiguration extends DataFlow::Config
   }
 }
 
+/**
+ * Data flow configuration for finding an operation with hardcoded 365 that will flow into
+ * a `FILEINFO` field.
+ */
+private module FiletimeYearArithmeticOperationCheckConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(Expr e, Operation op | e = source.asExpr() |
+      op.getAChild*().getValue().toInt() = 365 and
+      op.getAChild*() = e
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr, Expr e | e = sink.asExpr() |
+      dds instanceof PackedTimeType and
+      fa.getQualifier().getUnderlyingType() = dds and
+      fa.isModified() and
+      aexpr.getAChild() = fa and
+      aexpr.getChild(1).getAChild*() = e
+    )
+  }
+}
+
+module FiletimeYearArithmeticOperationCheckFlow =
+  DataFlow::Make<FiletimeYearArithmeticOperationCheckConfiguration>;
+
 /**
  * Taint configuration for finding an operation with hardcoded 365 that will flow into any known date/time field.
  */
-class PossibleYearArithmeticOperationCheckConfiguration extends TaintTracking::Configuration {
+deprecated class PossibleYearArithmeticOperationCheckConfiguration extends TaintTracking::Configuration {
   PossibleYearArithmeticOperationCheckConfiguration() {
     this = "PossibleYearArithmeticOperationCheckConfiguration"
   }
@@ -288,3 +329,46 @@ class PossibleYearArithmeticOperationCheckConfiguration extends TaintTracking::C
     )
   }
 }
+
+/**
+ * Taint configuration for finding an operation with hardcoded 365 that will flow into any known date/time field.
+ */
+private module PossibleYearArithmeticOperationCheckConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(Operation op | op = source.asConvertedExpr() |
+      op.getAChild*().getValue().toInt() = 365 and
+      (
+        not op.getParent() instanceof Expr or
+        op.getParent() instanceof Assignment
+      )
+    )
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    // flow from anything on the RHS of an assignment to a time/date structure to that
+    // assignment.
+    exists(StructLikeClass dds, FieldAccess fa, Assignment aexpr, Expr e |
+      e = node1.asExpr() and
+      fa = node2.asExpr()
+    |
+      (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and
+      fa.getQualifier().getUnderlyingType() = dds and
+      aexpr.getLValue() = fa and
+      aexpr.getRValue().getAChild*() = e
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr |
+      aexpr.getRValue() = sink.asConvertedExpr()
+    |
+      (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and
+      fa.getQualifier().getUnderlyingType() = dds and
+      fa.isModified() and
+      aexpr.getLValue() = fa
+    )
+  }
+}
+
+module PossibleYearArithmeticOperationCheckFlow =
+  TaintTracking::Make<PossibleYearArithmeticOperationCheckConfiguration>;

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql

@@ -29,21 +29,18 @@ where
       )
       or
       // If there is a data flow from the variable that was modified to a function that seems to check for leap year
-      exists(
-        VariableAccess source, ChecksForLeapYearFunctionCall fc, LeapYearCheckConfiguration config
-      |
+      exists(VariableAccess source, ChecksForLeapYearFunctionCall fc |
         source = var.getAnAccess() and
-        config.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(fc.getAnArgument()))
+        LeapYearCheckFlow::hasFlow(DataFlow::exprNode(source),
+          DataFlow::exprNode(fc.getAnArgument()))
       )
       or
       // If there is a data flow from the field that was modified to a function that seems to check for leap year
-      exists(
-        VariableAccess vacheck, YearFieldAccess yfacheck, ChecksForLeapYearFunctionCall fc,
-        LeapYearCheckConfiguration config
-      |
+      exists(VariableAccess vacheck, YearFieldAccess yfacheck, ChecksForLeapYearFunctionCall fc |
         vacheck = var.getAnAccess() and
         yfacheck.getQualifier() = vacheck and
-        config.hasFlow(DataFlow::exprNode(yfacheck), DataFlow::exprNode(fc.getAnArgument()))
+        LeapYearCheckFlow::hasFlow(DataFlow::exprNode(yfacheck),
+          DataFlow::exprNode(fc.getAnArgument()))
       )
       or
       // If there is a successor or predecessor that sets the month = 1

cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql

@@ -12,25 +12,25 @@
 import cpp
 import semmle.code.cpp.security.boostorg.asio.protocols
 
-class ExistsAnyFlowConfig extends DataFlow::Configuration {
-  ExistsAnyFlowConfig() { this = "ExistsAnyFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
+module ExistsAnyFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = source.asExpr())
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(BoostorgAsio::SslSetOptionsFunction f, FunctionCall fcSetOptions |
       f.getACallToThisFunction() = fcSetOptions and
       fcSetOptions.getQualifier() = sink.asExpr()
     )
   }
 }
 
+module ExistsAnyFlow = DataFlow::Make<ExistsAnyFlowConfig>;
+
 bindingset[flag]
 predicate isOptionSet(ConstructorCall cc, int flag, FunctionCall fcSetOptions) {
-  exists(ExistsAnyFlowConfig anyFlowConfig, VariableAccess contextSetOptions |
-    anyFlowConfig.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
+  exists(VariableAccess contextSetOptions |
+    ExistsAnyFlow::hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
     exists(BoostorgAsio::SslSetOptionsFunction f | f.getACallToThisFunction() = fcSetOptions |
       contextSetOptions = fcSetOptions.getQualifier() and
       forall(

cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql

@@ -13,7 +13,6 @@
 
 import cpp
 import semmle.code.cpp.ir.dataflow.DataFlow
-import semmle.code.cpp.ir.dataflow.DataFlow2
 
 predicate isCreateProcessFunction(FunctionCall call, int applicationNameIndex, int commandLineIndex) {
   call.getTarget().hasGlobalName("CreateProcessA") and
@@ -55,42 +54,40 @@ class CreateProcessFunctionCall extends FunctionCall {
 /**
  * Dataflow that detects a call to CreateProcess with a NULL value for lpApplicationName argument
  */
-class NullAppNameCreateProcessFunctionConfiguration extends DataFlow::Configuration {
-  NullAppNameCreateProcessFunctionConfiguration() {
-    this = "NullAppNameCreateProcessFunctionConfiguration"
-  }
-
-  override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue }
+module NullAppNameCreateProcessFunctionConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof NullValue }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(CreateProcessFunctionCall call, Expr val | val = sink.asExpr() |
       val = call.getArgument(call.getApplicationNameArgumentId())
     )
   }
 }
 
+module NullAppNameCreateProcessFunction =
+  DataFlow::Make<NullAppNameCreateProcessFunctionConfiguration>;
+
 /**
  * Dataflow that detects a call to CreateProcess with an unquoted commandLine argument
  */
-class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Configuration {
-  QuotedCommandInCreateProcessFunctionConfiguration() {
-    this = "QuotedCommandInCreateProcessFunctionConfiguration"
-  }
-
-  override predicate isSource(DataFlow2::Node source) {
+module QuotedCommandInCreateProcessFunctionConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     exists(string s |
       s = source.asExpr().getValue().toString() and
       not isQuotedOrNoSpaceApplicationNameOnCmd(s)
     )
   }
 
-  override predicate isSink(DataFlow2::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(CreateProcessFunctionCall call, Expr val | val = sink.asExpr() |
       val = call.getArgument(call.getCommandLineArgumentId())
     )
   }
 }
 
+module QuotedCommandInCreateProcessFunction =
+  DataFlow::Make<QuotedCommandInCreateProcessFunctionConfiguration>;
+
 bindingset[s]
 predicate isQuotedOrNoSpaceApplicationNameOnCmd(string s) {
   s.regexpMatch("\"([^\"])*\"[\\s\\S]*") // The first element (path) is quoted
@@ -100,14 +97,14 @@ predicate isQuotedOrNoSpaceApplicationNameOnCmd(string s) {
 
 from CreateProcessFunctionCall call, string msg1, string msg2
 where
-  exists(Expr appName, NullAppNameCreateProcessFunctionConfiguration nullAppConfig |
+  exists(Expr appName |
     appName = call.getArgument(call.getApplicationNameArgumentId()) and
-    nullAppConfig.hasFlowToExpr(appName) and
+    NullAppNameCreateProcessFunction::hasFlowToExpr(appName) and
     msg1 = call.toString() + " with lpApplicationName == NULL (" + appName + ")"
   ) and
-  exists(Expr cmd, QuotedCommandInCreateProcessFunctionConfiguration quotedConfig |
+  exists(Expr cmd |
     cmd = call.getArgument(call.getCommandLineArgumentId()) and
-    quotedConfig.hasFlowToExpr(cmd) and
+    QuotedCommandInCreateProcessFunction::hasFlowToExpr(cmd) and
     msg2 =
       " and with an unquoted lpCommandLine (" + cmd +
         ") introduces a security vulnerability if the path contains spaces."

cpp/ql/src/Security/CWE/CWE-497/PotentiallyExposedSystemData.ql

@@ -28,17 +28,15 @@ import cpp
 import semmle.code.cpp.ir.dataflow.TaintTracking
 import semmle.code.cpp.models.interfaces.FlowSource
 import semmle.code.cpp.security.OutputWrite
-import DataFlow::PathGraph
+import PotentiallyExposedSystemData::PathGraph
 import SystemData
 
-class PotentiallyExposedSystemDataConfiguration extends TaintTracking::Configuration {
-  PotentiallyExposedSystemDataConfiguration() { this = "PotentiallyExposedSystemDataConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) {
+module PotentiallyExposedSystemDataConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     source = any(SystemData sd | sd.isSensitive()).getAnExpr()
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(OutputWrite ow, Expr child | child = ow.getASource().getAChild*() |
       // Most sinks receive a pointer as an argument (for example `printf`),
       // and we use an indirect sink for those.
@@ -53,9 +51,10 @@ class PotentiallyExposedSystemDataConfiguration extends TaintTracking::Configura
   }
 }
 
-from
-  PotentiallyExposedSystemDataConfiguration config, DataFlow::PathNode source,
-  DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
+module PotentiallyExposedSystemData =
+  TaintTracking::Make<PotentiallyExposedSystemDataConfiguration>;
+
+from PotentiallyExposedSystemData::PathNode source, PotentiallyExposedSystemData::PathNode sink
+where PotentiallyExposedSystemData::hasFlowPath(source, sink)
 select sink, source, sink, "This operation potentially exposes sensitive system data from $@.",
   source, source.getNode().toString()