Merge pull request #16007 from MathiasVP/fix-tls-settings-misconfiguration

C++: Fix `cpp/boost/tls-settings-misconfiguration` FPs

Author: MathiasVP

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

@@ -12,34 +12,42 @@
 import cpp
 import semmle.code.cpp.security.boostorg.asio.protocols
 
+predicate isSourceImpl(DataFlow::Node source, ConstructorCall cc) {
+  exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = cc and cc = source.asExpr())
+}
+
+predicate isSinkImpl(DataFlow::Node sink, FunctionCall fcSetOptions) {
+  exists(BoostorgAsio::SslSetOptionsFunction f |
+    f.getACallToThisFunction() = fcSetOptions and
+    fcSetOptions.getQualifier() = sink.asIndirectExpr()
+  )
+}
+
 module ExistsAnyFlowConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = source.asExpr())
-  }
+  predicate isSource(DataFlow::Node source) { isSourceImpl(source, _) }
 
-  predicate isSink(DataFlow::Node sink) {
-    exists(BoostorgAsio::SslSetOptionsFunction f, FunctionCall fcSetOptions |
-      f.getACallToThisFunction() = fcSetOptions and
-      fcSetOptions.getQualifier() = sink.asExpr()
-    )
-  }
+  predicate isSink(DataFlow::Node sink) { isSinkImpl(sink, _) }
 }
 
 module ExistsAnyFlow = DataFlow::Global<ExistsAnyFlowConfig>;
 
 bindingset[flag]
 predicate isOptionSet(ConstructorCall cc, int flag, FunctionCall fcSetOptions) {
-  exists(VariableAccess contextSetOptions |
-    ExistsAnyFlow::flow(DataFlow::exprNode(cc), DataFlow::exprNode(contextSetOptions)) and
-    exists(BoostorgAsio::SslSetOptionsFunction f | f.getACallToThisFunction() = fcSetOptions |
-      contextSetOptions = fcSetOptions.getQualifier() and
-      forall(Expr optionArgument, Expr optionArgumentSource |
-        optionArgument = fcSetOptions.getArgument(0) and
-        BoostorgAsio::SslOptionFlow::flow(DataFlow::exprNode(optionArgumentSource),
-          DataFlow::exprNode(optionArgument))
-      |
-        optionArgument.getValue().toInt().bitShiftRight(16).bitAnd(flag) = flag
-      )
+  exists(
+    VariableAccess contextSetOptions, BoostorgAsio::SslSetOptionsFunction f, DataFlow::Node source,
+    DataFlow::Node sink
+  |
+    isSourceImpl(source, cc) and
+    isSinkImpl(sink, fcSetOptions) and
+    ExistsAnyFlow::flow(source, sink) and
+    f.getACallToThisFunction() = fcSetOptions and
+    contextSetOptions = fcSetOptions.getQualifier() and
+    forall(Expr optionArgument, Expr optionArgumentSource |
+      optionArgument = fcSetOptions.getArgument(0) and
+      BoostorgAsio::SslOptionFlow::flow(DataFlow::exprNode(optionArgumentSource),
+        DataFlow::exprNode(optionArgument))
+    |
+      optionArgument.getValue().toInt().bitShiftRight(16).bitAnd(flag) = flag
     )
   )
 }

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

@@ -5,12 +5,7 @@
 | test2.cpp:31:32:31:65 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | 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 | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | 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 | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | 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:38:35:38:98 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | test2.cpp:38:65:38:97 | sslv23 | sslv23 | test2.cpp:38:35:38:98 | call to context | no_sslv3 has not been set |
-| test2.cpp:38:35:38:98 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | test2.cpp:38:65:38:97 | sslv23 | sslv23 | test2.cpp:38:35:38:98 | call to context | no_tlsv1 has not been set |
-| test2.cpp:38:35:38:98 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | test2.cpp:38:65:38:97 | sslv23 | sslv23 | test2.cpp:38:35:38:98 | call to context | no_tlsv1_1 has not been set |
 | test2.cpp:45:35:45:98 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | 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:45:35:45:98 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | test2.cpp:45:65:45:97 | sslv23 | sslv23 | test2.cpp:45:35:45:98 | call to context | no_tlsv1 has not been set |
-| test2.cpp:45:35:45:98 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | test2.cpp:45:65:45:97 | sslv23 | sslv23 | test2.cpp:45:35:45:98 | call to context | no_tlsv1_1 has not been set |
 | test2.cpp:52:32:52:65 | call to context | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | 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 | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | 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 | This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@. | test2.cpp:52:32:52:64 | sslv23 | sslv23 | test2.cpp:52:32:52:65 | call to context | no_tlsv1_1 has not been set |

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

@@ -34,7 +34,7 @@ void bad2()
 
 void good3()
 {
-	// GOOD [FALSE POSITIVE]
+	// 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);
 }