C++: Make additional flow steps more uniform

Author: jketema

cpp/ql/src/experimental/Security/CWE/CWE-409/DecompressionBomb.qll

@@ -18,6 +18,9 @@ abstract class DecompressionFunction extends Function {
 /**
  * The Decompression Flow Steps, extend this class to define new decompression sinks.
  */
-abstract class DecompressionFlowStep extends Function {
+abstract class DecompressionFlowStep extends string {
+  bindingset[this]
+  DecompressionFlowStep() { any() }
+
   abstract predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2);
 }

cpp/ql/src/experimental/Security/CWE/CWE-409/DecompressionBombs.ql

@@ -26,8 +26,7 @@ module DecompressionTaintConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { isSink(_, sink) }
 
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(DecompressionFlowStep f).isAdditionalFlowStep(node1, node2) or
-    nextInAdditionalFlowStep(node1, node2)
+    any(DecompressionFlowStep s).isAdditionalFlowStep(node1, node2)
   }
 }
 

cpp/ql/src/experimental/Security/CWE/CWE-409/LibArchive.qll

@@ -20,11 +20,11 @@ class Archive_read_data_block extends DecompressionFunction {
 /**
  * The `archive_read_open_filename` function as a flow step.
  */
-class ReadOpenFunction extends DecompressionFlowStep {
-  ReadOpenFunction() { this.hasGlobalName("archive_read_open_filename") }
+class ReadOpenFunctionStep extends DecompressionFlowStep {
+  ReadOpenFunctionStep() { this = "ReadOpenFunction"}
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() = this |
+    exists(FunctionCall fc | fc.getTarget().hasGlobalName("archive_read_open_filename") |
       node1.asIndirectExpr() = fc.getArgument(1) and
       node2.asIndirectExpr() = fc.getArgument(0)
     )

cpp/ql/src/experimental/Security/CWE/CWE-409/MiniZip.qll

@@ -42,13 +42,13 @@ class UnzOpenFunction extends DecompressionFunction {
 /**
  * The `mz_zip_reader_open_file` and `mz_zip_reader_open_file_in_memory` functions as a flow step.
  */
-class ReaderOpenFunction extends DecompressionFlowStep {
-  ReaderOpenFunction() {
-    this.hasGlobalName(["mz_zip_reader_open_file_in_memory", "mz_zip_reader_open_file"])
-  }
+class ReaderOpenFunctionStep extends DecompressionFlowStep {
+  ReaderOpenFunctionStep() { this = "ReaderOpenFunctionStep" }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() = this |
+    exists(FunctionCall fc |
+      fc.getTarget().hasGlobalName(["mz_zip_reader_open_file_in_memory", "mz_zip_reader_open_file"])
+    |
       node1.asIndirectExpr() = fc.getArgument(1) and
       node2.asIndirectExpr() = fc.getArgument(0)
     )

cpp/ql/src/experimental/Security/CWE/CWE-409/ZSTD.qll

@@ -44,11 +44,11 @@ class ZstdDecompressUsingDdictFunction extends DecompressionFunction {
 /**
  * The `fopen_orDie` function as a flow step.
  */
-class FopenOrDieFunction extends DecompressionFlowStep {
-  FopenOrDieFunction() { this.hasGlobalName("fopen_orDie") }
+class FopenOrDieFunctionStep extends DecompressionFlowStep {
+  FopenOrDieFunctionStep() { this = "FopenOrDieFunctionStep" }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() = this |
+    exists(FunctionCall fc | fc.getTarget().hasGlobalName("fopen_orDie") |
       node1.asIndirectExpr() = fc.getArgument(0) and
       node2.asExpr() = fc
     )
@@ -58,11 +58,11 @@ class FopenOrDieFunction extends DecompressionFlowStep {
 /**
  * The `fread_orDie` function as a flow step.
  */
-class FreadOrDieFunction extends DecompressionFlowStep {
-  FreadOrDieFunction() { this.hasGlobalName("fread_orDie") }
+class FreadOrDieFunctionStep extends DecompressionFlowStep {
+  FreadOrDieFunctionStep() { this = "FreadOrDieFunctionStep" }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() = this |
+    exists(FunctionCall fc | fc.getTarget().hasGlobalName("fread_orDie") |
       node1.asIndirectExpr() = fc.getArgument(2) and
       node2.asIndirectExpr() = fc.getArgument(0)
     )

cpp/ql/src/experimental/Security/CWE/CWE-409/ZlibGzopen.qll

@@ -43,11 +43,11 @@ class GzReadFunction extends DecompressionFunction {
  *
  * `gzdopen(int fd, const char *mode)`
  */
-class GzdopenFunction extends DecompressionFlowStep {
-  GzdopenFunction() { this.hasGlobalName("gzdopen") }
+class GzdopenFunctionStep extends DecompressionFlowStep {
+  GzdopenFunctionStep() { this = "GzdopenFunctionStep" }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() = this |
+    exists(FunctionCall fc | fc.getTarget().hasGlobalName("gzdopen") |
       node1.asExpr() = fc.getArgument(0) and
       node2.asExpr() = fc
     )
@@ -59,11 +59,11 @@ class GzdopenFunction extends DecompressionFlowStep {
  *
  * `gzopen(const char *path, const char *mode)`
  */
-class GzopenFunction extends DecompressionFlowStep {
-  GzopenFunction() { this.hasGlobalName("gzopen") }
+class GzopenFunctionStep extends DecompressionFlowStep {
+  GzopenFunctionStep() { this = "GzopenFunctionStep" }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() = this |
+    exists(FunctionCall fc | fc.getTarget().hasGlobalName("gzopen") |
       node1.asIndirectExpr() = fc.getArgument(0) and
       node2.asExpr() = fc
     )

cpp/ql/src/experimental/Security/CWE/CWE-409/ZlibInflator.qll

@@ -19,16 +19,21 @@ class InflateFunction extends DecompressionFunction {
 }
 
 /**
- * The `next_in` member of a `z_stream` variable is used in flow steps.
+ * The `next_in` member of a `z_stream` variable is used in a flow steps.
  */
-predicate nextInAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(Variable nextInVar, VariableAccess zStreamAccess |
-    nextInVar.getDeclaringType().hasName("z_stream") and
-    nextInVar.hasName("next_in") and
-    zStreamAccess.getType().hasName("z_stream")
-  |
-    nextInVar.getAnAccess().getQualifier().(VariableAccess).getTarget() = zStreamAccess.getTarget() and
-    node1.asIndirectExpr() = nextInVar.getAnAssignedValue() and
-    node2.asExpr() = zStreamAccess
-  )
+class NextInMemberStep extends DecompressionFlowStep {
+  NextInMemberStep() { this = "NextInMemberStep" }
+
+  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(Variable nextInVar, VariableAccess zStreamAccess |
+      nextInVar.getDeclaringType().hasName("z_stream") and
+      nextInVar.hasName("next_in") and
+      zStreamAccess.getType().hasName("z_stream")
+    |
+      nextInVar.getAnAccess().getQualifier().(VariableAccess).getTarget() =
+        zStreamAccess.getTarget() and
+      node1.asIndirectExpr() = nextInVar.getAnAssignedValue() and
+      node2.asExpr() = zStreamAccess
+    )
+  }
 }