Merge pull request github#5185 from MathiasVP/block-integral-types-in-cgixss-query

C++: Add isBarrier to cpp/cgi-xss

Author: MathiasVP

cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

@@ -34,6 +34,10 @@ class Configuration extends TaintTrackingConfiguration {
   override predicate isSink(Element tainted) {
     exists(PrintStdoutCall call | call.getAnArgument() = tainted)
   }
+
+  override predicate isBarrier(Expr e) {
+    super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType
+  }
 }
 
 from QueryString query, Element printedArg, PathNode sourceNode, PathNode sinkNode

cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/CgiXss.expected

@@ -8,14 +8,14 @@ edges
 | search.c:22:24:22:28 | *query | search.c:23:39:23:43 | query |
 | search.c:22:24:22:28 | query | search.c:23:39:23:43 | query |
 | search.c:22:24:22:28 | query | search.c:23:39:23:43 | query |
-| search.c:41:21:41:26 | call to getenv | search.c:14:24:14:28 | *query |
-| search.c:41:21:41:26 | call to getenv | search.c:14:24:14:28 | *query |
-| search.c:41:21:41:26 | call to getenv | search.c:14:24:14:28 | query |
-| search.c:41:21:41:26 | call to getenv | search.c:14:24:14:28 | query |
-| search.c:41:21:41:26 | call to getenv | search.c:22:24:22:28 | *query |
-| search.c:41:21:41:26 | call to getenv | search.c:22:24:22:28 | *query |
-| search.c:41:21:41:26 | call to getenv | search.c:22:24:22:28 | query |
-| search.c:41:21:41:26 | call to getenv | search.c:22:24:22:28 | query |
+| search.c:51:21:51:26 | call to getenv | search.c:14:24:14:28 | *query |
+| search.c:51:21:51:26 | call to getenv | search.c:14:24:14:28 | *query |
+| search.c:51:21:51:26 | call to getenv | search.c:14:24:14:28 | query |
+| search.c:51:21:51:26 | call to getenv | search.c:14:24:14:28 | query |
+| search.c:51:21:51:26 | call to getenv | search.c:22:24:22:28 | *query |
+| search.c:51:21:51:26 | call to getenv | search.c:22:24:22:28 | *query |
+| search.c:51:21:51:26 | call to getenv | search.c:22:24:22:28 | query |
+| search.c:51:21:51:26 | call to getenv | search.c:22:24:22:28 | query |
 nodes
 | search.c:14:24:14:28 | *query | semmle.label | *query |
 | search.c:14:24:14:28 | query | semmle.label | query |
@@ -29,12 +29,12 @@ nodes
 | search.c:23:39:23:43 | query | semmle.label | query |
 | search.c:23:39:23:43 | query | semmle.label | query |
 | search.c:23:39:23:43 | query | semmle.label | query |
-| search.c:41:21:41:26 | call to getenv | semmle.label | call to getenv |
-| search.c:41:21:41:26 | call to getenv | semmle.label | call to getenv |
-| search.c:45:5:45:15 | Argument 0 | semmle.label | Argument 0 |
-| search.c:45:17:45:25 | Argument 0 indirection | semmle.label | Argument 0 indirection |
-| search.c:47:5:47:15 | Argument 0 | semmle.label | Argument 0 |
-| search.c:47:17:47:25 | Argument 0 indirection | semmle.label | Argument 0 indirection |
+| search.c:51:21:51:26 | call to getenv | semmle.label | call to getenv |
+| search.c:51:21:51:26 | call to getenv | semmle.label | call to getenv |
+| search.c:55:5:55:15 | Argument 0 | semmle.label | Argument 0 |
+| search.c:55:17:55:25 | Argument 0 indirection | semmle.label | Argument 0 indirection |
+| search.c:57:5:57:15 | Argument 0 | semmle.label | Argument 0 |
+| search.c:57:17:57:25 | Argument 0 indirection | semmle.label | Argument 0 indirection |
 #select
-| search.c:17:8:17:12 | query | search.c:41:21:41:26 | call to getenv | search.c:17:8:17:12 | query | Cross-site scripting vulnerability due to $@. | search.c:41:21:41:26 | call to getenv | this query data |
-| search.c:23:39:23:43 | query | search.c:41:21:41:26 | call to getenv | search.c:23:39:23:43 | query | Cross-site scripting vulnerability due to $@. | search.c:41:21:41:26 | call to getenv | this query data |
+| search.c:17:8:17:12 | query | search.c:51:21:51:26 | call to getenv | search.c:17:8:17:12 | query | Cross-site scripting vulnerability due to $@. | search.c:51:21:51:26 | call to getenv | this query data |
+| search.c:23:39:23:43 | query | search.c:51:21:51:26 | call to getenv | search.c:23:39:23:43 | query | Cross-site scripting vulnerability due to $@. | search.c:51:21:51:26 | call to getenv | this query data |

cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/search.c

@@ -26,7 +26,7 @@ void bad_server2(char* query) {
   puts(do_search(query));
 }
 
-void good_server(char* query) {
+void good_server1(char* query) {
   puts("<p>Query results for ");
   // GOOD: Escape HTML characters before adding to a page
   char* query_escaped = escape_html(query);
@@ -37,14 +37,25 @@ void good_server(char* query) {
   puts(do_search(query));
 }
 
+int sscanf(const char *s, const char *format, ...);
+
+void good_server2(char* query) {
+  puts("<p>Query results for ");
+  // GOOD: Only an integer is added to the page.
+  int i = 0;
+  sscanf(query, "value=%i", &i);
+  printf("\n<p>%i</p>\n", i);
+}
+
 int main(int argc, char** argv) {
   char* raw_query = getenv("QUERY_STRING");
-  if (strcmp("good", argv[0]) == 0) {
-    good_server(raw_query);
+  if (strcmp("good1", argv[0]) == 0) {
+    good_server1(raw_query);
   } else if (strcmp("bad1", argv[0]) == 0) {
     bad_server1(raw_query);
-  } else {
+  } else if (strcmp("bad2", argv[0]) == 0) {
     bad_server2(raw_query);
+  } else if (strcmp("good2", argv[0]) == 0) {
+    good_server2(raw_query);
   }
 }
-