From 13ae15b867a2bb8a1ffe2c49b40393a14dac4f14 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 13 Jan 2023 18:34:56 +0000 Subject: [PATCH 1/5] C++: Add tests for more edge cases. --- .../AuthenticationBypass.expected | 32 +++++++++++++++++++ .../semmle/AuthenticationBypass/test.cpp | 24 ++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/AuthenticationBypass.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/AuthenticationBypass.expected index ef28e08058d..fdc76862fe9 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/AuthenticationBypass.expected +++ b/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 | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/test.cpp index 8ca3514055b..72b9155cb84 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/test.cpp +++ b/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; + } +} + From 2f09f0e2c12b9515e902292663c3b8b9fae564b0 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 13 Jan 2023 18:47:18 +0000 Subject: [PATCH 2/5] C++: Turn the huge list into a predicate. --- .../CWE/CWE-290/AuthenticationBypass.ql | 74 +++---------------- 1 file changed, 12 insertions(+), 62 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql index 06c4ee5efe5..f0615b9979f 100644 --- a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql +++ b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql @@ -15,6 +15,17 @@ 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 @@ -23,68 +34,7 @@ predicate hardCodedAddressOrIP(StringLiteral txt) { s.matches("\"www.%\"") or s.matches("\"http:%\"") or 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\"") + s.regexpMatch("\".*\\." + getATopLevelDomain() + "\"") ) } From 316117f5c9f56b9a8a13378a4f7144fca98d10bc Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 13 Jan 2023 18:50:30 +0000 Subject: [PATCH 3/5] C++: Reduce number of regexps. --- cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql index f0615b9979f..40aae5e2cfa 100644 --- a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql +++ b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql @@ -34,7 +34,7 @@ predicate hardCodedAddressOrIP(StringLiteral txt) { s.matches("\"www.%\"") or s.matches("\"http:%\"") or s.matches("\"https:%\"") or - s.regexpMatch("\".*\\." + getATopLevelDomain() + "\"") + s.regexpMatch("\".*\\.(" + concat(getATopLevelDomain(), "|") + ")\"") ) } From 1a416884d4a42f02b39da95d15f22e69354a65e8 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 13 Jan 2023 20:27:07 +0000 Subject: [PATCH 4/5] C++: Do something similar with the other three cases. --- cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql index 40aae5e2cfa..6372e781879 100644 --- a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql +++ b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql @@ -31,9 +31,7 @@ predicate hardCodedAddressOrIP(StringLiteral txt) { // 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.matches("\"http:%\"") or - s.matches("\"https:%\"") or + s.regexpMatch("\"(www\\.|http:|https:).*\"") or s.regexpMatch("\".*\\.(" + concat(getATopLevelDomain(), "|") + ")\"") ) } From d628cc5ab815088dc8bd2090646ebd629fe58917 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 17 Jan 2023 14:37:19 +0000 Subject: [PATCH 5/5] Update cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql Co-authored-by: Mathias Vorreiter Pedersen --- cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql index 6372e781879..1c163eade27 100644 --- a/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql +++ b/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql @@ -32,7 +32,7 @@ predicate hardCodedAddressOrIP(StringLiteral txt) { s.regexpMatch("\"[0-9]+[.][0-9]+[.][0-9]+[.][0-9]+\"") or // Hard-coded addresses such as www.mycompany.com s.regexpMatch("\"(www\\.|http:|https:).*\"") or - s.regexpMatch("\".*\\.(" + concat(getATopLevelDomain(), "|") + ")\"") + s.regexpMatch("\".*\\.(" + strictconcat(getATopLevelDomain(), "|") + ")\"") ) }