Merge pull request github#12588 from jketema/boost-config

jketema · web-flow · commit c56c1cbb62ba · 2023-03-20T15:31:35.000+01:00
C++: Refactor `BoostorgAsio` to use `DataFlow::ConfigSig`
diff --git a/cpp/ql/lib/change-notes/2023-03-20-boost-deprecated-dataflow-configurations.md b/cpp/ql/lib/change-notes/2023-03-20-boost-deprecated-dataflow-configurations.md
@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The `SslContextCallAbstractConfig`, `SslContextCallConfig`, `SslContextCallBannedProtocolConfig`, `SslContextCallTls12ProtocolConfig`, `SslContextCallTls13ProtocolConfig`, `SslContextCallTlsProtocolConfig`, `SslContextFlowsToSetOptionConfig`, `SslOptionConfig` dataflow configurations from `BoostorgAsio` have been deprecated. Please use `SslContextCallConfigSig`, `SslContextCallMake`, `SslContextCallFlow`, `SslContextCallBannedProtocolFlow`, `SslContextCallTls12ProtocolFlow`, `SslContextCallTls13ProtocolFlow`, `SslContextCallTlsProtocolFlow`, `SslContextFlowsToSetOptionFlow`.
diff --git a/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll b/cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll
@@ -357,7 +357,7 @@ module BoostorgAsio {
    * Abstract class for flows of protocol values to the first argument of a context
    * constructor.
    */
-  abstract class SslContextCallAbstractConfig extends DataFlow::Configuration {
+  abstract deprecated class SslContextCallAbstractConfig extends DataFlow::Configuration {
     bindingset[this]
     SslContextCallAbstractConfig() { any() }
 
@@ -369,10 +369,43 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * Signature for flows of protocol values to the first argument of a context
+   * constructor.
+   */
+  signature module SslContextCallConfigSig {
+    /**
+     * Holds if `source` is a relevant data flow source.
+     */
+    predicate isSource(DataFlow::Node source);
+
+    /**
+     * Holds if `sink` is a relevant data flow sink.
+     */
+    default predicate isSink(DataFlow::Node sink) {
+      exists(ConstructorCall cc, SslContextClass c, Expr e | e = sink.asExpr() |
+        c.getAContructorCall() = cc and
+        cc.getArgument(0) = e
+      )
+    }
+  }
+
+  /**
+   * Constructs a standard data flow computation for protocol values to the first argument
+   * of a context constructor.
+   */
+  module SslContextCallMake<SslContextCallConfigSig Config> {
+    private module C implements DataFlow::ConfigSig {
+      import Config
+    }
+
+    import DataFlow::Make<C>
+  }
+
   /**
    * Any protocol value that flows to the first argument of a context constructor.
    */
-  class SslContextCallConfig extends SslContextCallAbstractConfig {
+  deprecated class SslContextCallConfig extends SslContextCallAbstractConfig {
     SslContextCallConfig() { this = "SslContextCallConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -383,10 +416,24 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * Any protocol value that flows to the first argument of a context constructor.
+   */
+  private module SslContextCallConfig implements SslContextCallConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(Expr e | e = source.asExpr() |
+        e.fromSource() and
+        not e.getLocation().getFile().toString().matches("%/boost/asio/%")
+      )
+    }
+  }
+
+  module SslContextCallFlow = SslContextCallMake<SslContextCallConfig>;
+
   /**
    * A banned protocol value that flows to the first argument of a context constructor.
    */
-  class SslContextCallBannedProtocolConfig extends SslContextCallAbstractConfig {
+  deprecated class SslContextCallBannedProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallBannedProtocolConfig() { this = "SslContextCallBannedProtocolConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -398,10 +445,25 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * A banned protocol value that flows to the first argument of a context constructor.
+   */
+  private module SslContextCallBannedProtocolConfig implements SslContextCallConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(Expr e | e = source.asExpr() |
+        e.fromSource() and
+        not e.getLocation().getFile().toString().matches("%/boost/asio/%") and
+        isExprBannedBoostProtocol(e)
+      )
+    }
+  }
+
+  module SslContextCallBannedProtocolFlow = SslContextCallMake<SslContextCallBannedProtocolConfig>;
+
   /**
    * A TLS 1.2 protocol value that flows to the first argument of a context constructor.
    */
-  class SslContextCallTls12ProtocolConfig extends SslContextCallAbstractConfig {
+  deprecated class SslContextCallTls12ProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallTls12ProtocolConfig() { this = "SslContextCallTls12ProtocolConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -413,10 +475,25 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * A TLS 1.2 protocol value that flows to the first argument of a context constructor.
+   */
+  private module SslContextCallTls12ProtocolConfig implements SslContextCallConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(Expr e | e = source.asExpr() |
+        e.fromSource() and
+        not e.getLocation().getFile().toString().matches("%/boost/asio/%") and
+        isExprTls12BoostProtocol(e)
+      )
+    }
+  }
+
+  module SslContextCallTls12ProtocolFlow = SslContextCallMake<SslContextCallTls12ProtocolConfig>;
+
   /**
    * A TLS 1.3 protocol value that flows to the first argument of a context constructor.
    */
-  class SslContextCallTls13ProtocolConfig extends SslContextCallAbstractConfig {
+  deprecated class SslContextCallTls13ProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallTls13ProtocolConfig() { this = "SslContextCallTls12ProtocolConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -428,10 +505,25 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * A TLS 1.3 protocol value that flows to the first argument of a context constructor.
+   */
+  private module SslContextCallTls13ProtocolConfig implements SslContextCallConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(Expr e | e = source.asExpr() |
+        e.fromSource() and
+        not e.getLocation().getFile().toString().matches("%/boost/asio/%") and
+        isExprTls13BoostProtocol(e)
+      )
+    }
+  }
+
+  module SslContextCallTls13ProtocolFlow = SslContextCallMake<SslContextCallTls13ProtocolConfig>;
+
   /**
    * A generic TLS protocol value that flows to the first argument of a context constructor.
    */
-  class SslContextCallTlsProtocolConfig extends SslContextCallAbstractConfig {
+  deprecated class SslContextCallTlsProtocolConfig extends SslContextCallAbstractConfig {
     SslContextCallTlsProtocolConfig() { this = "SslContextCallTlsProtocolConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -443,10 +535,25 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * A generic TLS protocol value that flows to the first argument of a context constructor.
+   */
+  private module SslContextCallTlsProtocolConfig implements SslContextCallConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(Expr e | e = source.asExpr() |
+        e.fromSource() and
+        not e.getLocation().getFile().toString().matches("%/boost/asio/%") and
+        isExprTlsBoostProtocol(e)
+      )
+    }
+  }
+
+  module SslContextCallTlsProtocolFlow = SslContextCallMake<SslContextCallTlsProtocolConfig>;
+
   /**
    * A context constructor call that flows to a call to `SetOptions()`.
    */
-  class SslContextFlowsToSetOptionConfig extends DataFlow::Configuration {
+  deprecated class SslContextFlowsToSetOptionConfig extends DataFlow::Configuration {
     SslContextFlowsToSetOptionConfig() { this = "SslContextFlowsToSetOptionConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -467,10 +574,34 @@ module BoostorgAsio {
     }
   }
 
+  /**
+   * A context constructor call that flows to a call to `SetOptions()`.
+   */
+  private module SslContextFlowsToSetOptionConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(SslContextClass c, ConstructorCall cc |
+        cc = source.asExpr() and
+        c.getAContructorCall() = cc
+      )
+    }
+
+    predicate isSink(DataFlow::Node sink) {
+      exists(FunctionCall fc, SslSetOptionsFunction f, Variable v, VariableAccess va |
+        va = sink.asExpr()
+      |
+        f.getACallToThisFunction() = fc and
+        v.getAnAccess() = va and
+        va = fc.getQualifier()
+      )
+    }
+  }
+
+  module SslContextFlowsToSetOptionFlow = DataFlow::Make<SslContextFlowsToSetOptionConfig>;
+
   /**
    * An option value that flows to the first parameter of a call to `SetOptions()`.
    */
-  class SslOptionConfig extends DataFlow::Configuration {
+  deprecated class SslOptionConfig extends DataFlow::Configuration {
     SslOptionConfig() { this = "SslOptionConfig" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -488,4 +619,26 @@ module BoostorgAsio {
       )
     }
   }
+
+  /**
+   * An option value that flows to the first parameter of a call to `SetOptions()`.
+   */
+  private module SslOptionConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      exists(Expr e | e = source.asExpr() |
+        e.fromSource() and
+        not e.getLocation().getFile().toString().matches("%/boost/asio/%")
+      )
+    }
+
+    predicate isSink(DataFlow::Node sink) {
+      exists(SslSetOptionsFunction f, FunctionCall call |
+        sink.asExpr() = call.getArgument(0) and
+        f.getACallToThisFunction() = call and
+        not sink.getLocation().getFile().toString().matches("%/boost/asio/%")
+      )
+    }
+  }
+
+  module SslOptionFlow = DataFlow::Make<SslOptionConfig>;
 }
diff --git a/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql b/cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql
@@ -33,13 +33,10 @@ predicate isOptionSet(ConstructorCall cc, int flag, FunctionCall fcSetOptions) {
     ExistsAnyFlow::hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
     exists(BoostorgAsio::SslSetOptionsFunction f | f.getACallToThisFunction() = fcSetOptions |
       contextSetOptions = fcSetOptions.getQualifier() and
-      forall(
-        Expr optionArgument, BoostorgAsio::SslOptionConfig optionArgConfig,
-        Expr optionArgumentSource
-      |
+      forall(Expr optionArgument, Expr optionArgumentSource |
         optionArgument = fcSetOptions.getArgument(0) and
-        optionArgConfig
-            .hasFlow(DataFlow::exprNode(optionArgumentSource), DataFlow::exprNode(optionArgument))
+        BoostorgAsio::SslOptionFlow::hasFlow(DataFlow::exprNode(optionArgumentSource),
+          DataFlow::exprNode(optionArgument))
       |
         optionArgument.getValue().toInt().bitShiftRight(16).bitAnd(flag) = flag
       )
@@ -50,11 +47,10 @@ predicate isOptionSet(ConstructorCall cc, int flag, FunctionCall fcSetOptions) {
 bindingset[flag]
 predicate isOptionNotSet(ConstructorCall cc, int flag) { not isOptionSet(cc, flag, _) }
 
-from
-  BoostorgAsio::SslContextCallTlsProtocolConfig configConstructor, Expr protocolSource,
-  Expr protocolSink, ConstructorCall cc, Expr e, string msg
+from Expr protocolSource, Expr protocolSink, ConstructorCall cc, Expr e, string msg
 where
-  configConstructor.hasFlow(DataFlow::exprNode(protocolSource), DataFlow::exprNode(protocolSink)) and
+  BoostorgAsio::SslContextCallTlsProtocolFlow::hasFlow(DataFlow::exprNode(protocolSource),
+    DataFlow::exprNode(protocolSink)) and
   cc.getArgument(0) = protocolSink and
   (
     BoostorgAsio::isExprSslV23BoostProtocol(protocolSource) and
diff --git a/cpp/ql/src/Likely Bugs/Protocols/UseOfDeprecatedHardcodedProtocol.ql b/cpp/ql/src/Likely Bugs/Protocols/UseOfDeprecatedHardcodedProtocol.ql
@@ -12,18 +12,15 @@
 import cpp
 import semmle.code.cpp.security.boostorg.asio.protocols
 
-from
-  BoostorgAsio::SslContextCallConfig config, Expr protocolSource, Expr protocolSink,
-  ConstructorCall cc
+from Expr protocolSource, Expr protocolSink, ConstructorCall cc
 where
-  config.hasFlow(DataFlow::exprNode(protocolSource), DataFlow::exprNode(protocolSink)) and
-  not exists(BoostorgAsio::SslContextCallTlsProtocolConfig tlsConfig |
-    tlsConfig.hasFlow(DataFlow::exprNode(protocolSource), DataFlow::exprNode(protocolSink))
-  ) and
+  BoostorgAsio::SslContextCallFlow::hasFlow(DataFlow::exprNode(protocolSource),
+    DataFlow::exprNode(protocolSink)) and
+  not BoostorgAsio::SslContextCallTlsProtocolFlow::hasFlow(DataFlow::exprNode(protocolSource),
+    DataFlow::exprNode(protocolSink)) and
   cc.getArgument(0) = protocolSink and
-  exists(BoostorgAsio::SslContextCallBannedProtocolConfig bannedConfig |
-    bannedConfig.hasFlow(DataFlow::exprNode(protocolSource), DataFlow::exprNode(protocolSink))
-  )
+  BoostorgAsio::SslContextCallBannedProtocolFlow::hasFlow(DataFlow::exprNode(protocolSource),
+    DataFlow::exprNode(protocolSink))
 select protocolSink, "Usage of $@ specifying a deprecated hardcoded protocol $@ in function $@.",
   cc, "boost::asio::ssl::context::context", protocolSource, protocolSource.toString(),
   cc.getEnclosingFunction(), cc.getEnclosingFunction().toString()

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: deprecated
 +---
 +* The `SslContextCallAbstractConfig`, `SslContextCallConfig`, `SslContextCallBannedProtocolConfig`, `SslContextCallTls12ProtocolConfig`, `SslContextCallTls13ProtocolConfig`, `SslContextCallTlsProtocolConfig`, `SslContextFlowsToSetOptionConfig`, `SslOptionConfig` dataflow configurations from `BoostorgAsio` have been deprecated. Please use `SslContextCallConfigSig`, `SslContextCallMake`, `SslContextCallFlow`, `SslContextCallBannedProtocolFlow`, `SslContextCallTls12ProtocolFlow`, `SslContextCallTls13ProtocolFlow`, `SslContextCallTlsProtocolFlow`, `SslContextFlowsToSetOptionFlow`.