Merge pull request #8227 from geoffw0/319improve

C++: Promote cpp/non-https-url

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

@@ -3,7 +3,7 @@
  * @description Non-HTTPS connections can be intercepted by third parties.
  * @kind path-problem
  * @problem.severity warning
- * @precision medium
+ * @precision high
  * @id cpp/non-https-url
  * @tags security
  *       external/cwe/cwe-319
@@ -12,6 +12,7 @@
 
 import cpp
 import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 import DataFlow::PathGraph
 
 /**
@@ -57,7 +58,12 @@ class HttpStringToUrlOpenConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node src) {
     // Sources are strings containing an HTTP URL not in a private domain.
-    src.asExpr() instanceof HttpStringLiteral
+    src.asExpr() instanceof HttpStringLiteral and
+    // block taint starting at `strstr`, which is likely testing an existing URL, rather than constructing an HTTP URL.
+    not exists(FunctionCall fc |
+      fc.getTarget().getName() = ["strstr", "strcasestr"] and
+      fc.getArgument(1) = globalValueNumber(src.asExpr()).getAnExpr()
+    )
   }
 
   override predicate isSink(DataFlow::Node sink) {

cpp/ql/src/change-notes/2022-02-24-non-https-url.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The "Failure to use HTTPS URLs" (`cpp/non-https-url`) has been improved reducing false positive results, and its precision has been increased to 'high'.

cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected

@@ -7,6 +7,8 @@ edges
 | test.cpp:40:11:40:17 | access to array | test.cpp:11:26:11:28 | url |
 | test.cpp:46:18:46:26 | http:// | test.cpp:49:11:49:16 | buffer |
 | test.cpp:49:11:49:16 | buffer | test.cpp:11:26:11:28 | url |
+| test.cpp:110:21:110:40 | http://example.com | test.cpp:121:11:121:13 | ptr |
+| test.cpp:121:11:121:13 | ptr | test.cpp:11:26:11:28 | url |
 nodes
 | test.cpp:11:26:11:28 | url | semmle.label | url |
 | test.cpp:15:30:15:32 | url | semmle.label | url |
@@ -17,9 +19,12 @@ nodes
 | test.cpp:40:11:40:17 | access to array | semmle.label | access to array |
 | test.cpp:46:18:46:26 | http:// | semmle.label | http:// |
 | test.cpp:49:11:49:16 | buffer | semmle.label | buffer |
+| test.cpp:110:21:110:40 | http://example.com | semmle.label | http://example.com |
+| test.cpp:121:11:121:13 | ptr | semmle.label | ptr |
 subpaths
 #select
 | test.cpp:28:10:28:29 | http://example.com | test.cpp:28:10:28:29 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
 | test.cpp:35:23:35:42 | http://example.com | test.cpp:35:23:35:42 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
 | test.cpp:36:26:36:45 | http://example.com | test.cpp:36:26:36:45 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
 | test.cpp:46:18:46:26 | http:// | test.cpp:46:18:46:26 | http:// | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
+| test.cpp:110:21:110:40 | http://example.com | test.cpp:110:21:110:40 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |

cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/test.cpp

@@ -58,3 +58,66 @@ void test()
 		openUrl(buffer);
 	}
 }
+
+typedef unsigned long size_t;
+int strncmp(const char *s1, const char *s2, size_t n);
+char* strstr(char* s1, const char* s2);
+
+void test2(const char *url)
+{
+	if (strncmp(url, "http://", 7)) // GOOD (or at least dubious; we are not constructing the URL)
+	{
+		openUrl(url);
+	}
+}
+
+void test3(char *url)
+{
+	char *ptr;
+
+	ptr = strstr(url, "https://"); // GOOD (https)
+	if (!ptr)
+	{
+		ptr = strstr(url, "http://"); // GOOD (we are not constructing the URL)
+	}
+
+	if (ptr)
+	{
+		openUrl(ptr);
+	}
+}
+
+void test4(char *url)
+{
+	const char *https_string = "https://"; // GOOD (https)
+	const char *http_string = "http://"; // GOOD (we are not constructing the URL)
+	char *ptr;
+
+	ptr = strstr(url, https_string);
+	if (!ptr)
+	{
+		ptr = strstr(url, http_string);
+	}
+
+	if (ptr)
+	{
+		openUrl(ptr);
+	}
+}
+
+void test5()
+{
+	char *url_string = "http://example.com"; // BAD
+	char *ptr;
+
+	ptr = strstr(url_string, "https://"); // GOOD (https)
+	if (!ptr)
+	{
+		ptr = strstr(url_string, "http://"); // GOOD (we are not constructing the URL here)
+	}
+
+	if (ptr)
+	{
+		openUrl(ptr);
+	}
+}