Merge pull request #2249 from geoffw0/tlsperf

CPP: TlsSettingsMisconfiguration.ql performance and cleanup

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

@@ -13,20 +13,22 @@ import semmle.code.cpp.security.boostorg.asio.protocols
 class ExistsAnyFlowConfig extends DataFlow::Configuration {
   ExistsAnyFlowConfig() { this = "ExistsAnyFlowConfig" }
 
-  override predicate isSource(DataFlow::Node source) { any() }
+  override predicate isSource(DataFlow::Node source) {
+    exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = source.asExpr())
+  }
 
-  override predicate isSink(DataFlow::Node sink) { any() }
+  override predicate isSink(DataFlow::Node sink) {
+    exists(BoostorgAsio::SslSetOptionsFunction f, FunctionCall fcSetOptions |
+      f.getACallToThisFunction() = fcSetOptions and
+      fcSetOptions.getQualifier() = sink.asExpr()
+    )
+  }
 }
 
 bindingset[flag]
 predicate isOptionSet(ConstructorCall cc, int flag, FunctionCall fcSetOptions) {
-  exists(
+  exists(ExistsAnyFlowConfig anyFlowConfig, VariableAccess contextSetOptions |
-    BoostorgAsio::SslContextFlowsToSetOptionConfig config, ExistsAnyFlowConfig testConfig,
+    anyFlowConfig.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
-    Expr optionsSink
-  |
-    config.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(optionsSink)) and
-    exists(VariableAccess contextSetOptions |
-      testConfig.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
     exists(BoostorgAsio::SslSetOptionsFunction f | f.getACallToThisFunction() = fcSetOptions |
       contextSetOptions = fcSetOptions.getQualifier() and
       forall(
@@ -41,48 +43,22 @@ predicate isOptionSet(ConstructorCall cc, int flag, FunctionCall fcSetOptions) {
       )
     )
   )
-  )
 }
 
 bindingset[flag]
 predicate isOptionNotSet(ConstructorCall cc, int flag) {
-  not exists(
+  not exists(FunctionCall fcSetOptions | isOptionSet(cc, flag, fcSetOptions))
-    BoostorgAsio::SslContextFlowsToSetOptionConfig config, ExistsAnyFlowConfig testConfig,
-    Expr optionsSink
-  |
-    config.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(optionsSink)) and
-    exists(VariableAccess contextSetOptions |
-      testConfig.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
-      exists(FunctionCall fcSetOptions, BoostorgAsio::SslSetOptionsFunction f |
-        f.getACallToThisFunction() = fcSetOptions
-      |
-        contextSetOptions = fcSetOptions.getQualifier() and
-        forall(
-          Expr optionArgument, BoostorgAsio::SslOptionConfig optionArgConfig,
-          Expr optionArgumentSource
-        |
-          optionArgument = fcSetOptions.getArgument(0) and
-          optionArgConfig
-              .hasFlow(DataFlow::exprNode(optionArgumentSource), DataFlow::exprNode(optionArgument))
-        |
-          optionArgument.getValue().toInt().bitShiftRight(16).bitAnd(flag) = flag
-        )
-      )
-    )
-  )
 }
 
 from
-  BoostorgAsio::SslContextCallTlsProtocolConfig configConstructor,
+  BoostorgAsio::SslContextCallTlsProtocolConfig configConstructor, Expr protocolSource,
-  BoostorgAsio::SslContextFlowsToSetOptionConfig config, Expr protocolSource, Expr protocolSink,
+  Expr protocolSink, ConstructorCall cc, Expr e, string msg
-  ConstructorCall cc, Expr e, string msg
 where
   configConstructor.hasFlow(DataFlow::exprNode(protocolSource), DataFlow::exprNode(protocolSink)) and
   cc.getArgument(0) = protocolSink and
   (
     BoostorgAsio::isExprSslV23BoostProtocol(protocolSource) and
-    not exists(Expr optionsSink |
+    not (
-      config.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(optionsSink)) and
       isOptionSet(cc, BoostorgAsio::getShiftedSslOptionsNoSsl3(), _) and
       isOptionSet(cc, BoostorgAsio::getShiftedSslOptionsNoTls1(), _) and
       isOptionSet(cc, BoostorgAsio::getShiftedSslOptionsNoTls1_1(), _) and
@@ -91,8 +67,7 @@ where
     or
     BoostorgAsio::isExprTlsBoostProtocol(protocolSource) and
     not BoostorgAsio::isExprSslV23BoostProtocol(protocolSource) and
-    not exists(Expr optionsSink |
+    not (
-      config.hasFlow(DataFlow::exprNode(cc), DataFlow::exprNode(optionsSink)) and
       isOptionSet(cc, BoostorgAsio::getShiftedSslOptionsNoTls1(), _) and
       isOptionSet(cc, BoostorgAsio::getShiftedSslOptionsNoTls1_1(), _) and
       isOptionNotSet(cc, BoostorgAsio::getShiftedSslOptionsNoTls1_2())

cpp/ql/test/query-tests/Likely Bugs/Protocols/boostorg/TlsSettingsMisconfiguration.expected

@@ -1,3 +1,14 @@
+| test2.cpp:15:32:15:33 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:15:32:15:33 | call to context | boost::asio::ssl::context::context | test2.cpp:14:40:14:72 | sslv23 | sslv23 | test2.cpp:15:32:15:33 | call to context | no_sslv3 has not been set |
+| test2.cpp:23:32:23:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:23:32:23:65 | call to context | boost::asio::ssl::context::context | test2.cpp:23:32:23:64 | sslv23 | sslv23 | test2.cpp:23:32:23:65 | call to context | no_sslv3 has not been set |
+| test2.cpp:23:32:23:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:23:32:23:65 | call to context | boost::asio::ssl::context::context | test2.cpp:23:32:23:64 | sslv23 | sslv23 | test2.cpp:23:32:23:65 | call to context | no_tlsv1 has not been set |
+| test2.cpp:23:32:23:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:23:32:23:65 | call to context | boost::asio::ssl::context::context | test2.cpp:23:32:23:64 | sslv23 | sslv23 | test2.cpp:23:32:23:65 | call to context | no_tlsv1_1 has not been set |
+| test2.cpp:31:32:31:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:31:32:31:65 | call to context | boost::asio::ssl::context::context | test2.cpp:31:32:31:64 | sslv23 | sslv23 | test2.cpp:31:32:31:65 | call to context | no_sslv3 has not been set |
+| test2.cpp:31:32:31:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:31:32:31:65 | call to context | boost::asio::ssl::context::context | test2.cpp:31:32:31:64 | sslv23 | sslv23 | test2.cpp:31:32:31:65 | call to context | no_tlsv1 has not been set |
+| test2.cpp:31:32:31:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:31:32:31:65 | call to context | boost::asio::ssl::context::context | test2.cpp:31:32:31:64 | sslv23 | sslv23 | test2.cpp:31:32:31:65 | call to context | no_tlsv1_1 has not been set |
+| test2.cpp:45:35:45:98 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:45:35:45:98 | call to context | boost::asio::ssl::context::context | test2.cpp:45:65:45:97 | sslv23 | sslv23 | test2.cpp:45:35:45:98 | call to context | no_sslv3 has not been set |
+| test2.cpp:52:32:52:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:52:32:52:65 | call to context | boost::asio::ssl::context::context | test2.cpp:52:32:52:64 | sslv23 | sslv23 | test2.cpp:52:32:52:65 | call to context | no_sslv3 has not been set |
+| test2.cpp:52:32:52:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:52:32:52:65 | call to context | boost::asio::ssl::context::context | test2.cpp:52:32:52:64 | sslv23 | sslv23 | test2.cpp:52:32:52:65 | call to context | no_tlsv1 has not been set |
+| test2.cpp:52:32:52:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test2.cpp:52:32:52:65 | call to context | boost::asio::ssl::context::context | test2.cpp:52:32:52:64 | sslv23 | sslv23 | test2.cpp:52:32:52:65 | call to context | no_tlsv1_1 has not been set |
 | test.cpp:25:32:25:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test.cpp:25:32:25:65 | call to context | boost::asio::ssl::context::context | test.cpp:25:32:25:64 | sslv23 | sslv23 | test.cpp:25:32:25:65 | call to context | no_sslv3 has not been set |
 | test.cpp:31:32:31:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test.cpp:31:32:31:65 | call to context | boost::asio::ssl::context::context | test.cpp:31:32:31:64 | sslv23 | sslv23 | test.cpp:31:32:31:65 | call to context | no_sslv3 has not been set |
 | test.cpp:31:32:31:65 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test.cpp:31:32:31:65 | call to context | boost::asio::ssl::context::context | test.cpp:31:32:31:64 | sslv23 | sslv23 | test.cpp:31:32:31:65 | call to context | no_tlsv1 has not been set |

cpp/ql/test/query-tests/Likely Bugs/Protocols/boostorg/test2.cpp

@@ -0,0 +1,55 @@
+#include "asio/boost_simulation.hpp"
+
+void good1()
+{
+	// GOOD
+	boost::asio::ssl::context::method m = boost::asio::ssl::context::sslv23;
+	boost::asio::ssl::context ctx(m);
+	ctx.set_options(boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1 | boost::asio::ssl::context::no_sslv3);
+}
+
+void bad1()
+{
+	// BAD: missing disable SSLv3
+	boost::asio::ssl::context::method m = boost::asio::ssl::context::sslv23;
+	boost::asio::ssl::context ctx(m);
+	ctx.set_options(boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1);
+}
+
+void good2()
+{
+	// GOOD [FALSE POSITIVE x 3]
+	boost::asio::ssl::context::options opts = boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1 | boost::asio::ssl::context::no_sslv3;
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::sslv23);
+	ctx.set_options(opts);
+}
+
+void bad2()
+{
+	// BAD: missing disable SSLv3 [WITH FALSE POSITIVE x 2]
+	boost::asio::ssl::context::options opts = boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1;
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::sslv23);
+	ctx.set_options(opts);
+}
+
+void good3()
+{
+	// GOOD
+	boost::asio::ssl::context *ctx = new boost::asio::ssl::context(boost::asio::ssl::context::sslv23);
+	ctx->set_options(boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1 | boost::asio::ssl::context::no_sslv3);
+}
+
+void bad3()
+{
+	// BAD: missing disable SSLv3
+	boost::asio::ssl::context *ctx = new boost::asio::ssl::context(boost::asio::ssl::context::sslv23);
+	ctx->set_options(boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1);
+}
+
+void bad4()
+{
+	// BAD: missing disable SSLv3
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::sslv23);
+}
+
+