Merge pull request #11891 from geoffw0/authbypass

C++: Fix issue with cpp/user-controlled-bypass

cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql

@@ -15,76 +15,24 @@
 import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
 import TaintedWithPath
 
+string getATopLevelDomain() {
+  result =
+    [
+      "com", "ru", "net", "org", "de", "jp", "uk", "br", "pl", "in", "it", "fr", "au", "info", "nl",
+      "cn", "ir", "es", "cz", "biz", "ca", "eu", "ua", "kr", "za", "co", "gr", "ro", "se", "tw",
+      "vn", "mx", "ch", "tr", "at", "be", "hu", "tv", "dk", "me", "ar", "us", "no", "sk", "fi",
+      "id", "cl", "nz", "by", "xyz", "pt", "ie", "il", "kz", "my", "hk", "lt", "cc", "sg", "io",
+      "edu", "gov"
+    ]
+}
+
 predicate hardCodedAddressOrIP(StringLiteral txt) {
   exists(string s | s = txt.getValueText() |
     // Hard-coded ip addresses, such as 127.0.0.1
     s.regexpMatch("\"[0-9]+[.][0-9]+[.][0-9]+[.][0-9]+\"") or
     // Hard-coded addresses such as www.mycompany.com
-    s.matches("\"www.%\"") or
+    s.regexpMatch("\"(www\\.|http:|https:).*\"") or
-    s.matches("\"http:%\"") or
+    s.regexpMatch("\".*\\.(" + strictconcat(getATopLevelDomain(), "|") + ")\"")
-    s.matches("\"https:%\"") or
-    s.matches("\"%.com\"") or
-    s.matches("\"%.ru\"") or
-    s.matches("\"%.net\"") or
-    s.matches("\"%.org\"") or
-    s.matches("\"%.de\"") or
-    s.matches("\"%.jp\"") or
-    s.matches("\"%.uk\"") or
-    s.matches("\"%.br\"") or
-    s.matches("\"%.pl\"") or
-    s.matches("\"%.in\"") or
-    s.matches("\"%.it\"") or
-    s.matches("\"%.fr\"") or
-    s.matches("\"%.au\"") or
-    s.matches("\"%.info\"") or
-    s.matches("\"%.nl\"") or
-    s.matches("\"%.cn\"") or
-    s.matches("\"%.ir\"") or
-    s.matches("\"%.es\"") or
-    s.matches("\"%.cz\"") or
-    s.matches("\"%.biz\"") or
-    s.matches("\"%.ca\"") or
-    s.matches("\"%.eu\"") or
-    s.matches("\"%.ua\"") or
-    s.matches("\"%.kr\"") or
-    s.matches("\"%.za\"") or
-    s.matches("\"%.co\"") or
-    s.matches("\"%.gr\"") or
-    s.matches("\"%.ro\"") or
-    s.matches("\"%.se\"") or
-    s.matches("\"%.tw\"") or
-    s.matches("\"%.vn\"") or
-    s.matches("\"%.mx\"") or
-    s.matches("\"%.ch\"") or
-    s.matches("\"%.tr\"") or
-    s.matches("\"%.at\"") or
-    s.matches("\"%.be\"") or
-    s.matches("\"%.hu\"") or
-    s.matches("\"%.tv\"") or
-    s.matches("\"%.dk\"") or
-    s.matches("\"%.me\"") or
-    s.matches("\"%.ar\"") or
-    s.matches("\"%.us\"") or
-    s.matches("\"%.no\"") or
-    s.matches("\"%.sk\"") or
-    s.matches("\"%.fi\"") or
-    s.matches("\"%.id\"") or
-    s.matches("\"%.cl\"") or
-    s.matches("\"%.nz\"") or
-    s.matches("\"%.by\"") or
-    s.matches("\"%.xyz\"") or
-    s.matches("\"%.pt\"") or
-    s.matches("\"%.ie\"") or
-    s.matches("\"%.il\"") or
-    s.matches("\"%.kz\"") or
-    s.matches("\"%.my\"") or
-    s.matches("\"%.hk\"") or
-    s.matches("\"%.lt\"") or
-    s.matches("\"%.cc\"") or
-    s.matches("\"%.sg\"") or
-    s.matches("\"%.io\"") or
-    s.matches("\"%.edu\"") or
-    s.matches("\"%.gov\"")
   )
 }
 

cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/AuthenticationBypass.expected

@@ -17,6 +17,24 @@ edges
 | test.cpp:38:25:38:42 | (const char *)... | test.cpp:42:14:42:20 | address |
 | test.cpp:38:25:38:42 | (const char *)... | test.cpp:42:14:42:20 | address |
 | test.cpp:38:25:38:42 | (const char *)... | test.cpp:42:14:42:20 | address indirection |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address indirection |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address indirection |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address |
+| test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address indirection |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:52:14:52:20 | address |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:52:14:52:20 | address |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:52:14:52:20 | address indirection |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:56:14:56:20 | address |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:56:14:56:20 | address |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:56:14:56:20 | address indirection |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:60:14:60:20 | address |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:60:14:60:20 | address |
+| test.cpp:49:25:49:42 | (const char *)... | test.cpp:60:14:60:20 | address indirection |
 subpaths
 nodes
 | test.cpp:16:25:16:30 | call to getenv | semmle.label | call to getenv |
@@ -34,7 +52,21 @@ nodes
 | test.cpp:42:14:42:20 | address | semmle.label | address |
 | test.cpp:42:14:42:20 | address | semmle.label | address |
 | test.cpp:42:14:42:20 | address indirection | semmle.label | address indirection |
+| test.cpp:49:25:49:30 | call to getenv | semmle.label | call to getenv |
+| test.cpp:49:25:49:42 | (const char *)... | semmle.label | (const char *)... |
+| test.cpp:52:14:52:20 | address | semmle.label | address |
+| test.cpp:52:14:52:20 | address | semmle.label | address |
+| test.cpp:52:14:52:20 | address indirection | semmle.label | address indirection |
+| test.cpp:56:14:56:20 | address | semmle.label | address |
+| test.cpp:56:14:56:20 | address | semmle.label | address |
+| test.cpp:56:14:56:20 | address indirection | semmle.label | address indirection |
+| test.cpp:60:14:60:20 | address | semmle.label | address |
+| test.cpp:60:14:60:20 | address | semmle.label | address |
+| test.cpp:60:14:60:20 | address indirection | semmle.label | address indirection |
 #select
 | test.cpp:20:7:20:12 | call to strcmp | test.cpp:16:25:16:30 | call to getenv | test.cpp:20:14:20:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:16:25:16:30 | call to getenv | call to getenv |
 | test.cpp:31:7:31:12 | call to strcmp | test.cpp:27:25:27:30 | call to getenv | test.cpp:31:14:31:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:27:25:27:30 | call to getenv | call to getenv |
 | test.cpp:42:7:42:12 | call to strcmp | test.cpp:38:25:38:30 | call to getenv | test.cpp:42:14:42:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:38:25:38:30 | call to getenv | call to getenv |
+| test.cpp:52:7:52:12 | call to strcmp | test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:30 | call to getenv | call to getenv |
+| test.cpp:56:7:56:12 | call to strcmp | test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:30 | call to getenv | call to getenv |
+| test.cpp:60:7:60:12 | call to strcmp | test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:30 | call to getenv | call to getenv |

cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/test.cpp

@@ -43,3 +43,27 @@ void processRequest3()
     isServer = 1;
   }
 }
+
+void processRequest4() 
+{
+  const char *address = getenv("SERVERIP");
+  bool cond = false;
+
+  if (strcmp(address, "127.0.0.1")) { cond = true; } // BAD
+  if (strcmp(address, "127_0_0_1")) { cond = true; } // GOOD (not an IP)
+  if (strcmp(address, "127.0.0")) { cond = true; } // GOOD (not an IP)
+  if (strcmp(address, "127.0.0.0.1")) { cond = true; } // GOOD (not an IP)
+  if (strcmp(address, "http://mycompany")) { cond = true; } // BAD
+  if (strcmp(address, "http_//mycompany")) { cond = true; } // GOOD (not an address)
+  if (strcmp(address, "htt://mycompany")) { cond = true; } // GOOD (not an address)
+  if (strcmp(address, "httpp://mycompany")) { cond = true; } // GOOD (not an address)
+  if (strcmp(address, "mycompany.com")) { cond = true; } // BAD
+  if (strcmp(address, "mycompany_com")) { cond = true; } // GOOD (not an address)
+  if (strcmp(address, "mycompany.c")) { cond = true; } // GOOD (not an address)
+  if (strcmp(address, "mycompany.comm")) { cond = true; } // GOOD (not an address)
+
+  if (cond) {
+    isServer = 1;
+  }
+}
+