Merge pull request #2376 from geoffw0/qhelpms2

CPP: Recommendations and examples for TlsSettingsMisconfiguration.qhelp and UseOfDeprecatedHardCodedProtocol.qhelp

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

@@ -6,6 +6,17 @@
   <p>Using the TLS or SSLv23 protocol from the boost::asio library, but not disabling deprecated protocols may expose the software to known vulnerabilities or permit weak encryption algorithms to be used.  Disabling the minimum-recommended protocols is also flagged.</p>
 </overview>
 
+<recommendation>
+<p>When using the TLS or SSLv23 protocol, set the <code>no_tlsv1</code> and <code>no_tlsv1_1</code> options, but do not set <code>no_tlsv1_2</code>. When using the SSLv23 protocol, also set the <code>no_sslv3</code> option.</p>
+</recommendation>
+
+<example>
+<p>In the following example, the <code>no_tlsv1_1</code> option has not been set.  Use of TLS 1.1 is not recommended.</p>
+<sample src="TlsSettingsMisconfigurationBad.cpp"/>
+<p>In the corrected example, the <code>no_tlsv1</code> and <code>no_tlsv1_1</code> options have both been set, ensuring the use of TLS 1.2 or later.</p>
+<sample src="TlsSettingsMisconfigurationGood.cpp"/>
+</example>
+
 <references>
   <li>
     <a href="https://www.boost.org/doc/libs/1_71_0/doc/html/boost_asio.html">Boost.Asio documentation</a>.

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

@@ -0,0 +1,8 @@
+
+void useTLS_bad()
+{
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::tls);
+	ctx.set_options(boost::asio::ssl::context::no_tlsv1); // BAD: missing no_tlsv1_1
+
+	// ...
+}

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

@@ -0,0 +1,8 @@
+
+void useTLS_good()
+{
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::tls);
+	ctx.set_options(boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1); // GOOD
+
+	// ...
+}

cpp/ql/src/Likely Bugs/Protocols/UseOfDeprecatedHardcodedProtocol.qhelp

@@ -4,13 +4,22 @@
 <qhelp>
 <overview>
   <p>Using boost::asio library but specifying a deprecated hardcoded protocol.</p>
-  <p>Using a deprecated hardcoded protocol instead of negotiting would lock your application to a protocol that has known vulnerabilities or weaknesses.</p>
 </overview>
 
+<recommendation>
+<p>Only use modern protocols such as TLS 1.2 or TLS 1.3.</p>
+</recommendation>
+
+<example>
+<p>In the following example, the <code>sslv2</code> protocol is specified.  This protocol is out of date and its use is not recommended.</p>
+<sample src="UseOfDeprecatedHardcodedProtocolBad.cpp"/>
+<p>In the corrected example, the <code>tlsv13</code> protocol is used instead.</p>
+<sample src="UseOfDeprecatedHardcodedProtocolGood.cpp"/>
+</example>
+
 <references>
   <li>
     <a href="https://www.boost.org/doc/libs/1_71_0/doc/html/boost_asio.html">Boost.Asio documentation</a>.
   </li>
 </references>
 </qhelp>
-

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

@@ -0,0 +1,7 @@
+
+void useProtocol_bad()
+{
+	boost::asio::ssl::context ctx_sslv2(boost::asio::ssl::context::sslv2); // BAD: outdated protocol
+
+	// ...
+}

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

@@ -0,0 +1,7 @@
+
+void useProtocol_good()
+{
+	boost::asio::ssl::context cxt_tlsv13(boost::asio::ssl::context::tlsv13);
+
+	// ...
+}

cpp/ql/src/Security/CWE/CWE-457/ConditionallyUninitializedVariable.ql

@@ -1,7 +1,7 @@
 /**
  * @name Conditionally uninitialized variable
- * @description When an initialization function is used to initialize a local variable, but the
- *              returned status code is not checked, the variable may be left in an uninitialized
+ * @description An initialization function is used to initialize a local variable, but the
+ *              returned status code is not checked. The variable may be left in an uninitialized
  *              state, and reading the variable may result in undefined behavior.
  * @kind problem
  * @problem.severity warning

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

@@ -9,6 +9,7 @@
 | 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 |
+| test3.cpp:7:32:7:62 | call to context | Usage of $@ with protocol $@ is not configured correctly: The option $@. | test3.cpp:7:32:7:62 | call to context | boost::asio::ssl::context::context | test3.cpp:7:32:7:61 | tls | tls | test3.cpp:7:32:7:62 | 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/test.cpp

@@ -65,13 +65,13 @@ void TestHardcodedProtocols()
 
 	////////////////////// Hardcoded algorithms
 
-	boost::asio::ssl::context cxt_tlsv12(boost::asio::ssl::context::tlsv12);			// BUG
-	boost::asio::ssl::context cxt_tlsv12c(boost::asio::ssl::context::tlsv12_client);	// BUG
-	boost::asio::ssl::context cxt_tlsv12s(boost::asio::ssl::context::tlsv12_server);	// BUG
+	boost::asio::ssl::context cxt_tlsv12(boost::asio::ssl::context::tlsv12);
+	boost::asio::ssl::context cxt_tlsv12c(boost::asio::ssl::context::tlsv12_client);
+	boost::asio::ssl::context cxt_tlsv12s(boost::asio::ssl::context::tlsv12_server);
 
-	boost::asio::ssl::context cxt_tlsv13(boost::asio::ssl::context::tlsv13);			// BUG
-	boost::asio::ssl::context cxt_tlsv13c(boost::asio::ssl::context::tlsv13_client);	// BUG
-	boost::asio::ssl::context cxt_tlsv13s(boost::asio::ssl::context::tlsv13_server);	// BUG
+	boost::asio::ssl::context cxt_tlsv13(boost::asio::ssl::context::tlsv13);
+	boost::asio::ssl::context cxt_tlsv13c(boost::asio::ssl::context::tlsv13_client);
+	boost::asio::ssl::context cxt_tlsv13s(boost::asio::ssl::context::tlsv13_server);
 }
 
 void InterProceduralTest(boost::asio::ssl::context::method m)
@@ -100,11 +100,11 @@ void TestHardcodedProtocols_inter()
 
 	////////////////////// Hardcoded algorithms
 
-	InterProceduralTest(boost::asio::ssl::context::tlsv12);			// BUG
-	InterProceduralTest(boost::asio::ssl::context::tlsv12_client);	// BUG
-	InterProceduralTest(boost::asio::ssl::context::tlsv12_server);	// BUG
+	InterProceduralTest(boost::asio::ssl::context::tlsv12);
+	InterProceduralTest(boost::asio::ssl::context::tlsv12_client);
+	InterProceduralTest(boost::asio::ssl::context::tlsv12_server);
 
-	InterProceduralTest(boost::asio::ssl::context::tlsv13);			// BUG
-	InterProceduralTest(boost::asio::ssl::context::tlsv13_client);	// BUG
-	InterProceduralTest(boost::asio::ssl::context::tlsv13_server);	// BUG
+	InterProceduralTest(boost::asio::ssl::context::tlsv13);
+	InterProceduralTest(boost::asio::ssl::context::tlsv13_client);
+	InterProceduralTest(boost::asio::ssl::context::tlsv13_server);
 }

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

@@ -0,0 +1,19 @@
+#include "asio/boost_simulation.hpp"
+
+// examples from the qhelp...
+
+void useTLS_bad()
+{
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::tls);
+	ctx.set_options(boost::asio::ssl::context::no_tlsv1); // BAD: missing no_tlsv1_1
+
+	// ...
+}
+
+void useTLS_good()
+{
+	boost::asio::ssl::context ctx(boost::asio::ssl::context::tls);
+	ctx.set_options(boost::asio::ssl::context::no_tlsv1 | boost::asio::ssl::context::no_tlsv1_1); // GOOD
+
+	// ...
+}