From ce8912ddcc38f553b29ca6f072d3d2a1ef059437 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 13:36:16 +0100 Subject: [PATCH 1/9] Test: Handle 'problems' result set as an alias for '#select' --- .../util/test/InlineExpectationsTest.qll | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/shared/util/codeql/util/test/InlineExpectationsTest.qll b/shared/util/codeql/util/test/InlineExpectationsTest.qll index 415450c16032..8dde42b51678 100644 --- a/shared/util/codeql/util/test/InlineExpectationsTest.qll +++ b/shared/util/codeql/util/test/InlineExpectationsTest.qll @@ -583,6 +583,9 @@ private string expectationPattern() { ) } +/** Gets the string `#select` or `problems`, which are equivalent result sets for a `problem` or `path-problem` query. */ +private string mainResultSet() { result = ["#select", "problems"] } + /** * Provides logic for creating a `@kind test-postprocess` query that checks * inline test expectations using `$ Alert` markers. @@ -650,8 +653,8 @@ module TestPostProcessing { */ private string getSourceTag(int row) { getQueryKind() = "path-problem" and - exists(string loc | queryResults("#select", row, 2, loc) | - if queryResults("#select", row, 0, loc) then result = "Alert" else result = "Source" + exists(string loc | queryResults(mainResultSet(), row, 2, loc) | + if queryResults(mainResultSet(), row, 0, loc) then result = "Alert" else result = "Source" ) } @@ -663,8 +666,8 @@ module TestPostProcessing { */ private string getSinkTag(int row) { getQueryKind() = "path-problem" and - exists(string loc | queryResults("#select", row, 4, loc) | - if queryResults("#select", row, 0, loc) then result = "Alert" else result = "Sink" + exists(string loc | queryResults(mainResultSet(), row, 4, loc) | + if queryResults(mainResultSet(), row, 0, loc) then result = "Alert" else result = "Sink" ) } @@ -717,8 +720,8 @@ module TestPostProcessing { ) { getQueryKind() = "path-problem" and exists(string loc | - queryResults("#select", row, 2, loc) and - queryResults("#select", row, 3, element) and + queryResults(mainResultSet(), row, 2, loc) and + queryResults(mainResultSet(), row, 3, element) and tag = getSourceTag(row) and value = "" and Input2::getRelativeUrl(location) = loc @@ -757,8 +760,8 @@ module TestPostProcessing { ) { getQueryKind() = "path-problem" and exists(string loc | - queryResults("#select", row, 4, loc) and - queryResults("#select", row, 5, element) and + queryResults(mainResultSet(), row, 4, loc) and + queryResults(mainResultSet(), row, 5, element) and tag = getSinkTag(row) and Input2::getRelativeUrl(location) = loc ) @@ -767,8 +770,8 @@ module TestPostProcessing { private predicate hasAlert(int row, Input::Location location, string element, string tag) { getQueryKind() = ["problem", "path-problem"] and exists(string loc | - queryResults("#select", row, 0, loc) and - queryResults("#select", row, 2, element) and + queryResults(mainResultSet(), row, 0, loc) and + queryResults(mainResultSet(), row, 2, element) and tag = "Alert" and Input2::getRelativeUrl(location) = loc and not hasPathProblemSource(row, location, _, _, _) and From a83508a828e405c4fae0b76ea98be8d3bb185016 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 13:42:56 +0100 Subject: [PATCH 2/9] JS: Port IncompleteHostNameRegExt test --- .../IncompleteHostnameRegExp.expected | 8 +++ .../IncompleteHostnameRegExp.qlref | 3 +- .../tst-IncompleteHostnameRegExp.js | 62 +++++++++---------- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected index 8341636994e4..cbf5e905d51f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected @@ -1,3 +1,4 @@ +problems | tst-IncompleteHostnameRegExp.js:3:3:3:28 | ^http:\\/\\/test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | here | | tst-IncompleteHostnameRegExp.js:5:3:5:28 | ^http:\\/\\/test.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | here | | tst-IncompleteHostnameRegExp.js:6:3:6:42 | ^http:\\/\\/test.(example-a\|example-b).com | This regular expression has an unescaped '.' before '(example-a\|example-b).com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | here | @@ -26,3 +27,10 @@ | tst-IncompleteHostnameRegExp.js:55:14:55:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | here | | tst-IncompleteHostnameRegExp.js:59:5:59:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:59:2:59:32 | /^(foo. ... ever)$/ | here | | tst-IncompleteHostnameRegExp.js:61:18:61:41 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:61:17:61:42 | "^http: ... le.com" | here | +testFailures +| tst-IncompleteHostnameRegExp.js:9:45:9:54 | // $ Alert | Missing result: Alert | +| tst-IncompleteHostnameRegExp.js:10:3:10:36 | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | Unexpected result: Alert | +| tst-IncompleteHostnameRegExp.js:17:35:17:44 | // $ Alert | Missing result: Alert | +| tst-IncompleteHostnameRegExp.js:36:43:36:52 | // $ Alert | Missing result: Alert | +| tst-IncompleteHostnameRegExp.js:42:69:42:78 | // $ Alert | Missing result: Alert | +| tst-IncompleteHostnameRegExp.js:59:5:59:20 | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | Unexpected result: Alert | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref index e818d9472521..d1afa3858252 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref @@ -1 +1,2 @@ -Security/CWE-020/IncompleteHostnameRegExp.ql \ No newline at end of file +query: Security/CWE-020/IncompleteHostnameRegExp.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js index 51c2c9914209..2963d2b2fdc0 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js @@ -1,62 +1,62 @@ (function() { - /^http:\/\/example.com/; // OK - /^http:\/\/test.example.com/; // NOT OK - /^http:\/\/test\\.example.com/; // OK - /^http:\/\/test.example.net/; // NOT OK - /^http:\/\/test.(example-a|example-b).com/; // NOT OK - /^http:\/\/(.+).example.com\//; // NOT OK - /^http:\/\/(\\.+)\\.example.com/; // OK - /^http:\/\/(?:.+)\\.test\\.example.com\//; // NOT OK - /^http:\/\/test.example.com\/(?:.*)/; // OK - new RegExp("^http://test.example.com"); // NOT OK - if (s.match("^http://test.example.com")) {} // NOT OK + /^http:\/\/example.com/; + /^http:\/\/test.example.com/; // $ Alert + /^http:\/\/test\\.example.com/; + /^http:\/\/test.example.net/; // $ Alert + /^http:\/\/test.(example-a|example-b).com/; // $ Alert + /^http:\/\/(.+).example.com\//; // $ Alert + /^http:\/\/(\\.+)\\.example.com/; + /^http:\/\/(?:.+)\\.test\\.example.com\//; // $ Alert + /^http:\/\/test.example.com\/(?:.*)/; + new RegExp("^http://test.example.com"); // $ Alert + if (s.match("^http://test.example.com")) {} // $ Alert function id(e) { return e; } - new RegExp(id(id(id("^http://test.example.com")))); // NOT OK + new RegExp(id(id(id("^http://test.example.com")))); // $ Alert - new RegExp(`test.example.com$`); // NOT OK + new RegExp(`test.example.com$`); // $ Alert - let hostname = '^test.example.com'; // NOT OK + let hostname = '^test.example.com'; // $ Alert new RegExp(`${hostname}$`); - let domain = { hostname: 'test.example.com$' }; // NOT OK + let domain = { hostname: 'test.example.com$' }; // $ Alert new RegExp(domain.hostname); function convert1(domain) { return new RegExp(domain.hostname); } - convert1({ hostname: 'test.example.com$' }); // NOT OK + convert1({ hostname: 'test.example.com$' }); // $ Alert - let domains = [ { hostname: 'test.example.com$' } ]; // NOT OK + let domains = [ { hostname: 'test.example.com$' } ]; // $ Alert function convert2(domain) { return new RegExp(domain.hostname); } domains.map(d => convert2(d)); - /^(.+\.(?:example-a|example-b)\.com)\//; // NOT OK - /^(https?:)?\/\/((service|www).)?example.com(?=$|\/)/; // NOT OK - /^(http|https):\/\/www.example.com\/p\/f\//; // NOT OK - /^(http:\/\/sub.example.com\/)/g; // NOT OK - /^https?:\/\/api.example.com/; // NOT OK - new RegExp('^http://localhost:8000|' + '^https?://.+\\.example\\.com/'); // NOT OK - new RegExp('^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)'); // NOT OK - /^https:\/\/[a-z]*.example.com$/; // NOT OK - RegExp('^protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); // NOT OK + /^(.+\.(?:example-a|example-b)\.com)\//; // $ Alert + /^(https?:)?\/\/((service|www).)?example.com(?=$|\/)/; // $ Alert + /^(http|https):\/\/www.example.com\/p\/f\//; // $ Alert + /^(http:\/\/sub.example.com\/)/g; // $ Alert + /^https?:\/\/api.example.com/; // $ Alert + new RegExp('^http://localhost:8000|' + '^https?://.+\\.example\\.com/'); // $ Alert + new RegExp('^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)'); // $ Alert + /^https:\/\/[a-z]*.example.com$/; // $ Alert + RegExp('^protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); // $ Alert /^(example.dev|example.com)/; // OK - new RegExp('^http://localhost:8000|' + '^https?://.+.example\\.com/'); // NOT OK + new RegExp('^http://localhost:8000|' + '^https?://.+.example\\.com/'); // $ Alert var primary = 'example.com$'; - new RegExp('test.' + primary); // NOT OK, but not detected + new RegExp('test.' + primary); // $ MISSING: Alert - new RegExp('test.' + 'example.com$'); // NOT OK + new RegExp('test.' + 'example.com$'); // $ Alert - new RegExp('^http://test\.example.com'); // NOT OK + new RegExp('^http://test\.example.com'); // $ Alert /^http:\/\/(..|...)\.example\.com\/index\.html/; // OK, wildcards are intentional /^http:\/\/.\.example\.com\/index\.html/; // OK, the wildcard is intentional /^(foo.example\.com|whatever)$/; // kinda OK - one disjunction doesn't even look like a hostname - if (s.matchAll("^http://test.example.com")) {} // NOT OK + if (s.matchAll("^http://test.example.com")) {} // $ Alert }); From 48f7a58d01694a388ce77b857789df125a2e000c Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 13:54:05 +0100 Subject: [PATCH 3/9] JS: Update IncompleteHostnameRegExp test to match reality --- .../IncompleteHostnameRegExp.expected | 8 -------- .../tst-IncompleteHostnameRegExp.js | 12 ++++++------ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected index cbf5e905d51f..8341636994e4 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected @@ -1,4 +1,3 @@ -problems | tst-IncompleteHostnameRegExp.js:3:3:3:28 | ^http:\\/\\/test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | here | | tst-IncompleteHostnameRegExp.js:5:3:5:28 | ^http:\\/\\/test.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | here | | tst-IncompleteHostnameRegExp.js:6:3:6:42 | ^http:\\/\\/test.(example-a\|example-b).com | This regular expression has an unescaped '.' before '(example-a\|example-b).com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | here | @@ -27,10 +26,3 @@ problems | tst-IncompleteHostnameRegExp.js:55:14:55:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | here | | tst-IncompleteHostnameRegExp.js:59:5:59:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:59:2:59:32 | /^(foo. ... ever)$/ | here | | tst-IncompleteHostnameRegExp.js:61:18:61:41 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:61:17:61:42 | "^http: ... le.com" | here | -testFailures -| tst-IncompleteHostnameRegExp.js:9:45:9:54 | // $ Alert | Missing result: Alert | -| tst-IncompleteHostnameRegExp.js:10:3:10:36 | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | Unexpected result: Alert | -| tst-IncompleteHostnameRegExp.js:17:35:17:44 | // $ Alert | Missing result: Alert | -| tst-IncompleteHostnameRegExp.js:36:43:36:52 | // $ Alert | Missing result: Alert | -| tst-IncompleteHostnameRegExp.js:42:69:42:78 | // $ Alert | Missing result: Alert | -| tst-IncompleteHostnameRegExp.js:59:5:59:20 | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | Unexpected result: Alert | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js index 2963d2b2fdc0..f822f7ed5479 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js @@ -6,15 +6,15 @@ /^http:\/\/test.(example-a|example-b).com/; // $ Alert /^http:\/\/(.+).example.com\//; // $ Alert /^http:\/\/(\\.+)\\.example.com/; - /^http:\/\/(?:.+)\\.test\\.example.com\//; // $ Alert - /^http:\/\/test.example.com\/(?:.*)/; + /^http:\/\/(?:.+)\\.test\\.example.com\//; // $ MISSING: Alert (TODO) + /^http:\/\/test.example.com\/(?:.*)/; // $ SPURIOUS: Alert (TODO) new RegExp("^http://test.example.com"); // $ Alert if (s.match("^http://test.example.com")) {} // $ Alert function id(e) { return e; } new RegExp(id(id(id("^http://test.example.com")))); // $ Alert - new RegExp(`test.example.com$`); // $ Alert + new RegExp(`test.example.com$`); // $ MISSING: Alert (TODO) let hostname = '^test.example.com'; // $ Alert new RegExp(`${hostname}$`); @@ -33,13 +33,13 @@ } domains.map(d => convert2(d)); - /^(.+\.(?:example-a|example-b)\.com)\//; // $ Alert + /^(.+\.(?:example-a|example-b)\.com)\//; // $ MISSING: Alert (TODO) /^(https?:)?\/\/((service|www).)?example.com(?=$|\/)/; // $ Alert /^(http|https):\/\/www.example.com\/p\/f\//; // $ Alert /^(http:\/\/sub.example.com\/)/g; // $ Alert /^https?:\/\/api.example.com/; // $ Alert new RegExp('^http://localhost:8000|' + '^https?://.+\\.example\\.com/'); // $ Alert - new RegExp('^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)'); // $ Alert + new RegExp('^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)'); // $ MISSING: Alert (TODO) /^https:\/\/[a-z]*.example.com$/; // $ Alert RegExp('^protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); // $ Alert @@ -56,7 +56,7 @@ /^http:\/\/(..|...)\.example\.com\/index\.html/; // OK, wildcards are intentional /^http:\/\/.\.example\.com\/index\.html/; // OK, the wildcard is intentional - /^(foo.example\.com|whatever)$/; // kinda OK - one disjunction doesn't even look like a hostname + /^(foo.example\.com|whatever)$/; // $ SPURIOUS: Alert (TODO) (kinda OK - one disjunction doesn't even look like a hostname) if (s.matchAll("^http://test.example.com")) {} // $ Alert }); From 563471dd52934f39d13130773d3f2a326aa066f2 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 14:02:36 +0100 Subject: [PATCH 4/9] JS: Triage discrepancies and update test --- .../IncompleteHostnameRegExp.expected | 55 ++++++++++--------- .../tst-IncompleteHostnameRegExp.js | 17 +++--- 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected index 8341636994e4..90d4e925d21e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected @@ -1,28 +1,29 @@ | tst-IncompleteHostnameRegExp.js:3:3:3:28 | ^http:\\/\\/test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | here | -| tst-IncompleteHostnameRegExp.js:5:3:5:28 | ^http:\\/\\/test.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | here | -| tst-IncompleteHostnameRegExp.js:6:3:6:42 | ^http:\\/\\/test.(example-a\|example-b).com | This regular expression has an unescaped '.' before '(example-a\|example-b).com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | here | -| tst-IncompleteHostnameRegExp.js:7:3:7:30 | ^http:\\/\\/(.+).example.com\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:7:2:7:31 | /^http: ... .com\\// | here | -| tst-IncompleteHostnameRegExp.js:7:3:7:30 | ^http:\\/\\/(.+).example.com\\/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example.com' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:7:2:7:31 | /^http: ... .com\\// | here | -| tst-IncompleteHostnameRegExp.js:10:3:10:36 | ^http:\\/\\/test.example.com\\/(?:.*) | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:10:2:10:37 | /^http: ... (?:.*)/ | here | -| tst-IncompleteHostnameRegExp.js:11:14:11:37 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:11:13:11:38 | "^http: ... le.com" | here | -| tst-IncompleteHostnameRegExp.js:12:15:12:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:12:14:12:39 | "^http: ... le.com" | here | -| tst-IncompleteHostnameRegExp.js:15:23:15:46 | ^http://test.example.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:15:13:15:50 | id(id(i ... com"))) | here | -| tst-IncompleteHostnameRegExp.js:19:18:19:34 | ^test.example.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:20:13:20:26 | `${hostname}$` | here | -| tst-IncompleteHostnameRegExp.js:22:28:22:44 | test.example.com$ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:23:13:23:27 | domain.hostname | here | -| tst-IncompleteHostnameRegExp.js:28:24:28:40 | test.example.com$ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:26:21:26:35 | domain.hostname | here | -| tst-IncompleteHostnameRegExp.js:30:31:30:47 | test.example.com$ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:32:21:32:35 | domain.hostname | here | -| tst-IncompleteHostnameRegExp.js:37:3:37:53 | ^(https?:)?\\/\\/((service\|www).)?example.com(?=$\|\\/) | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:37:2:37:54 | /^(http ... =$\|\\/)/ | here | -| tst-IncompleteHostnameRegExp.js:38:3:38:43 | ^(http\|https):\\/\\/www.example.com\\/p\\/f\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:38:2:38:44 | /^(http ... p\\/f\\// | here | -| tst-IncompleteHostnameRegExp.js:39:5:39:30 | http:\\/\\/sub.example.com\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:39:2:39:33 | /^(http ... om\\/)/g | here | -| tst-IncompleteHostnameRegExp.js:40:3:40:29 | ^https?:\\/\\/api.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:40:2:40:30 | /^https ... le.com/ | here | -| tst-IncompleteHostnameRegExp.js:41:42:41:48 | ^https?://.+\\.example\\.com/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:41:13:41:71 | '^http: ... \\.com/' | here | -| tst-IncompleteHostnameRegExp.js:43:3:43:32 | ^https:\\/\\/[a-z]*.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:43:2:43:33 | /^https ... e.com$/ | here | -| tst-IncompleteHostnameRegExp.js:44:32:44:45 | .+.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | here | -| tst-IncompleteHostnameRegExp.js:44:47:44:62 | .+.example-a.com | This regular expression has an unescaped '.' before 'example-a.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | here | -| tst-IncompleteHostnameRegExp.js:44:64:44:79 | .+.example-b.com | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | here | -| tst-IncompleteHostnameRegExp.js:48:42:48:47 | ^https?://.+.example\\.com/ | This regular expression has an unescaped '.' before 'example\\.com/', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:48:13:48:69 | '^http: ... \\.com/' | here | -| tst-IncompleteHostnameRegExp.js:48:42:48:47 | ^https?://.+.example\\.com/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:48:13:48:69 | '^http: ... \\.com/' | here | -| tst-IncompleteHostnameRegExp.js:53:14:53:35 | test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:53:13:53:36 | 'test.' ... e.com$' | here | -| tst-IncompleteHostnameRegExp.js:55:14:55:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | here | -| tst-IncompleteHostnameRegExp.js:59:5:59:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:59:2:59:32 | /^(foo. ... ever)$/ | here | -| tst-IncompleteHostnameRegExp.js:61:18:61:41 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:61:17:61:42 | "^http: ... le.com" | here | +| tst-IncompleteHostnameRegExp.js:6:3:6:28 | ^http:\\/\\/test.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:6:2:6:29 | /^http: ... le.net/ | here | +| tst-IncompleteHostnameRegExp.js:7:3:7:42 | ^http:\\/\\/test.(example-a\|example-b).com | This regular expression has an unescaped '.' before '(example-a\|example-b).com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:7:2:7:43 | /^http: ... b).com/ | here | +| tst-IncompleteHostnameRegExp.js:8:3:8:30 | ^http:\\/\\/(.+).example.com\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:8:2:8:31 | /^http: ... .com\\// | here | +| tst-IncompleteHostnameRegExp.js:8:3:8:30 | ^http:\\/\\/(.+).example.com\\/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example.com' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:8:2:8:31 | /^http: ... .com\\// | here | +| tst-IncompleteHostnameRegExp.js:10:3:10:39 | ^http:\\/\\/(?:.+)\\.test\\.example.com\\/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example.com' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:10:2:10:40 | /^http: ... .com\\// | here | +| tst-IncompleteHostnameRegExp.js:11:3:11:36 | ^http:\\/\\/test.example.com\\/(?:.*) | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:11:2:11:37 | /^http: ... (?:.*)/ | here | +| tst-IncompleteHostnameRegExp.js:12:14:12:37 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:12:13:12:38 | "^http: ... le.com" | here | +| tst-IncompleteHostnameRegExp.js:13:15:13:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:13:14:13:39 | "^http: ... le.com" | here | +| tst-IncompleteHostnameRegExp.js:16:23:16:46 | ^http://test.example.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:16:13:16:50 | id(id(i ... com"))) | here | +| tst-IncompleteHostnameRegExp.js:20:18:20:34 | ^test.example.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:21:13:21:26 | `${hostname}$` | here | +| tst-IncompleteHostnameRegExp.js:23:28:23:44 | test.example.com$ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:24:13:24:27 | domain.hostname | here | +| tst-IncompleteHostnameRegExp.js:29:24:29:40 | test.example.com$ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:27:21:27:35 | domain.hostname | here | +| tst-IncompleteHostnameRegExp.js:31:31:31:47 | test.example.com$ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:33:21:33:35 | domain.hostname | here | +| tst-IncompleteHostnameRegExp.js:38:3:38:53 | ^(https?:)?\\/\\/((service\|www).)?example.com(?=$\|\\/) | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:38:2:38:54 | /^(http ... =$\|\\/)/ | here | +| tst-IncompleteHostnameRegExp.js:39:3:39:43 | ^(http\|https):\\/\\/www.example.com\\/p\\/f\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:39:2:39:44 | /^(http ... p\\/f\\// | here | +| tst-IncompleteHostnameRegExp.js:40:5:40:30 | http:\\/\\/sub.example.com\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:40:2:40:33 | /^(http ... om\\/)/g | here | +| tst-IncompleteHostnameRegExp.js:41:3:41:29 | ^https?:\\/\\/api.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:41:2:41:30 | /^https ... le.com/ | here | +| tst-IncompleteHostnameRegExp.js:42:42:42:48 | ^https?://.+\\.example\\.com/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:42:13:42:71 | '^http: ... \\.com/' | here | +| tst-IncompleteHostnameRegExp.js:44:3:44:32 | ^https:\\/\\/[a-z]*.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:44:2:44:33 | /^https ... e.com$/ | here | +| tst-IncompleteHostnameRegExp.js:45:32:45:45 | .+.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:45:9:45:101 | '^proto ... ernal)' | here | +| tst-IncompleteHostnameRegExp.js:45:47:45:62 | .+.example-a.com | This regular expression has an unescaped '.' before 'example-a.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:45:9:45:101 | '^proto ... ernal)' | here | +| tst-IncompleteHostnameRegExp.js:45:64:45:79 | .+.example-b.com | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:45:9:45:101 | '^proto ... ernal)' | here | +| tst-IncompleteHostnameRegExp.js:49:42:49:47 | ^https?://.+.example\\.com/ | This regular expression has an unescaped '.' before 'example\\.com/', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:49:13:49:69 | '^http: ... \\.com/' | here | +| tst-IncompleteHostnameRegExp.js:49:42:49:47 | ^https?://.+.example\\.com/ | This regular expression has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.js:49:13:49:69 | '^http: ... \\.com/' | here | +| tst-IncompleteHostnameRegExp.js:54:14:54:35 | test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:54:13:54:36 | 'test.' ... e.com$' | here | +| tst-IncompleteHostnameRegExp.js:56:14:56:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:56:13:56:39 | '^http: ... le.com' | here | +| tst-IncompleteHostnameRegExp.js:60:5:60:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:60:2:60:32 | /^(foo. ... ever)$/ | here | +| tst-IncompleteHostnameRegExp.js:62:18:62:41 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:62:17:62:42 | "^http: ... le.com" | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js index f822f7ed5479..320175ab1d42 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js @@ -1,20 +1,21 @@ (function() { /^http:\/\/example.com/; /^http:\/\/test.example.com/; // $ Alert - /^http:\/\/test\\.example.com/; + /^http:\/\/test\.example.com/; // OK - escaped dot + /^http:\/\/test\\.example.com/; // OK - contains actual backslash, so not really a hostname /^http:\/\/test.example.net/; // $ Alert /^http:\/\/test.(example-a|example-b).com/; // $ Alert /^http:\/\/(.+).example.com\//; // $ Alert - /^http:\/\/(\\.+)\\.example.com/; - /^http:\/\/(?:.+)\\.test\\.example.com\//; // $ MISSING: Alert (TODO) - /^http:\/\/test.example.com\/(?:.*)/; // $ SPURIOUS: Alert (TODO) + /^http:\/\/(\.+)\.example.com/; + /^http:\/\/(?:.+)\.test\.example.com\//; // $ Alert + /^http:\/\/test.example.com\/(?:.*)/; // $ Alert new RegExp("^http://test.example.com"); // $ Alert if (s.match("^http://test.example.com")) {} // $ Alert function id(e) { return e; } new RegExp(id(id(id("^http://test.example.com")))); // $ Alert - new RegExp(`test.example.com$`); // $ MISSING: Alert (TODO) + new RegExp(`test.example.com$`); // $ MISSING: Alert let hostname = '^test.example.com'; // $ Alert new RegExp(`${hostname}$`); @@ -33,13 +34,13 @@ } domains.map(d => convert2(d)); - /^(.+\.(?:example-a|example-b)\.com)\//; // $ MISSING: Alert (TODO) + /^(.+\.(?:example-a|example-b)\.com)\//; // $ MISSING: Alert /^(https?:)?\/\/((service|www).)?example.com(?=$|\/)/; // $ Alert /^(http|https):\/\/www.example.com\/p\/f\//; // $ Alert /^(http:\/\/sub.example.com\/)/g; // $ Alert /^https?:\/\/api.example.com/; // $ Alert new RegExp('^http://localhost:8000|' + '^https?://.+\\.example\\.com/'); // $ Alert - new RegExp('^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)'); // $ MISSING: Alert (TODO) + new RegExp('^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)'); /^https:\/\/[a-z]*.example.com$/; // $ Alert RegExp('^protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); // $ Alert @@ -56,7 +57,7 @@ /^http:\/\/(..|...)\.example\.com\/index\.html/; // OK, wildcards are intentional /^http:\/\/.\.example\.com\/index\.html/; // OK, the wildcard is intentional - /^(foo.example\.com|whatever)$/; // $ SPURIOUS: Alert (TODO) (kinda OK - one disjunction doesn't even look like a hostname) + /^(foo.example\.com|whatever)$/; // $ Alert (but kinda OK - one disjunction doesn't even look like a hostname) if (s.matchAll("^http://test.example.com")) {} // $ Alert }); From 95e20a045ba648bb2f423e55bef3805375e9f8ed Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 14:11:36 +0100 Subject: [PATCH 5/9] JS: Port IncompleteUrlSchemeCheck test --- .../IncompleteUrlSchemeCheck.expected | 6 ++++ .../IncompleteUrlSchemeCheck.js | 34 +++++++++---------- .../IncompleteUrlSchemeCheck.qlref | 3 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected index 5f563bfcd8f5..a2bb67b67b95 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected @@ -1,3 +1,4 @@ +#select | IncompleteUrlSchemeCheck.js:5:9:5:35 | u.start ... ript:") | This check does not consider data: and vbscript:. | | IncompleteUrlSchemeCheck.js:16:9:16:39 | badProt ... otocol) | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:23:9:23:43 | badProt ... scheme) | This check does not consider vbscript:. | @@ -13,3 +14,8 @@ | IncompleteUrlSchemeCheck.js:104:6:104:39 | /^(java ... scheme) | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:110:12:112:29 | url // ... :/, "") | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:124:11:124:34 | url.rep ... :/, "") | This check does not consider vbscript:. | +testFailures +| IncompleteUrlSchemeCheck.js:94:10:94:15 | This check does not consider vbscript:. | Unexpected result: Alert | +| IncompleteUrlSchemeCheck.js:95:25:95:34 | // $ Alert | Missing result: Alert | +| IncompleteUrlSchemeCheck.js:110:12:112:29 | This check does not consider vbscript:. | Unexpected result: Alert | +| IncompleteUrlSchemeCheck.js:110:17:110:26 | // $ Alert | Missing result: Alert | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js index 197e45e7b5dd..65dfb679edc9 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js @@ -2,7 +2,7 @@ import * as dummy from 'dummy'; function sanitizeUrl(url) { let u = decodeURI(url).trim().toLowerCase(); - if (u.startsWith("javascript:")) // NOT OK + if (u.startsWith("javascript:")) // $ Alert return "about:blank"; return url; } @@ -13,28 +13,28 @@ let badProtocolsGood = ['javascript:', 'data:', 'vbscript:']; function test2(url) { let protocol = new URL(url).protocol; - if (badProtocols.includes(protocol)) // NOT OK + if (badProtocols.includes(protocol)) // $ Alert return "about:blank"; return url; } function test3(url) { let scheme = goog.uri.utils.getScheme(url); - if (badProtocolNoColon.includes(scheme)) // NOT OK + if (badProtocolNoColon.includes(scheme)) // $ Alert return "about:blank"; return url; } function test4(url) { let scheme = url.split(':')[0]; - if (badProtocolNoColon.includes(scheme)) // NOT OK + if (badProtocolNoColon.includes(scheme)) // $ Alert return "about:blank"; return url; } function test5(url) { let scheme = url.split(':')[0]; - if (scheme === "javascript") // NOT OK + if (scheme === "javascript") // $ Alert return "about:blank"; return url; } @@ -48,35 +48,35 @@ function test6(url) { function test7(url) { let scheme = url.split(/:/)[0]; - if (scheme === "javascript") // NOT OK + if (scheme === "javascript") // $ Alert return "about:blank"; return url; } function test8(url) { let scheme = goog.uri.utils.getScheme(url); - if ("javascript|data".split("|").indexOf(scheme) !== -1) // NOT OK + if ("javascript|data".split("|").indexOf(scheme) !== -1) // $ Alert return "about:blank"; return url; } function test9(url) { let scheme = goog.uri.utils.getScheme(url); - if ("javascript" === scheme || "data" === scheme) // NOT OK + if ("javascript" === scheme || "data" === scheme) // $ Alert return "about:blank"; return url; } function test10(url) { let scheme = goog.uri.utils.getScheme(url); - if (/^(javascript|data)$/.exec(scheme) !== null) // NOT OK + if (/^(javascript|data)$/.exec(scheme) !== null) // $ Alert return "about:blank"; return url; } function test11(url) { let scheme = goog.uri.utils.getScheme(url); - if (/^(javascript|data)$/.exec(scheme) === null) // NOT OK + if (/^(javascript|data)$/.exec(scheme) === null) // $ Alert return url; return "about:blank"; } @@ -84,7 +84,7 @@ function test11(url) { function test12(url) { let scheme = goog.uri.utils.getScheme(url); - if (!/^(javascript|data)$/.exec(scheme)) // NOT OK + if (!/^(javascript|data)$/.exec(scheme)) // $ Alert return url; return "about:blank"; } @@ -92,7 +92,7 @@ function test12(url) { function test13(url) { let scheme = goog.uri.utils.getScheme(url); switch (scheme) { - case "javascript": // NOT OK + case "javascript": // $ Alert case "data": return "about:blank"; default: @@ -101,13 +101,13 @@ function test13(url) { } function test14(url) { let scheme = goog.uri.utils.getScheme(url); - if (/^(javascript|data)$/.exec(scheme)) // NOT OK + if (/^(javascript|data)$/.exec(scheme)) // $ Alert return "about:blank"; return url; } function chain1(url) { - return url // NOT OK + return url // $ Alert .replace(/javascript:/, "") .replace(/data:/, ""); } @@ -121,10 +121,10 @@ function chain2(url) { function chain3(url) { url = url.replace(/javascript:/, "") - url = url.replace(/data:/, ""); // NOT OK + url = url.replace(/data:/, ""); // $ Alert return url; } function chain4(url) { - return url.replace(/(javascript|data):/, ""); // NOT OK - but not flagged [INCONSISTENCY] -} \ No newline at end of file + return url.replace(/(javascript|data):/, ""); // $ MISSING: Alert +} diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.qlref index 99a744188b50..0c088087e994 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.qlref +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.qlref @@ -1 +1,2 @@ -Security/CWE-020/IncompleteUrlSchemeCheck.ql \ No newline at end of file +query: Security/CWE-020/IncompleteUrlSchemeCheck.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql From 6b4be13a8e47a3be4d5418fe143de7f6c424b430 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 14:12:54 +0100 Subject: [PATCH 6/9] JS: Move annotations to the correct line --- .../IncompleteUrlSchemeCheck.expected | 8 +------- .../IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js | 8 ++++---- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected index a2bb67b67b95..92b5410f2c83 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected @@ -1,4 +1,3 @@ -#select | IncompleteUrlSchemeCheck.js:5:9:5:35 | u.start ... ript:") | This check does not consider data: and vbscript:. | | IncompleteUrlSchemeCheck.js:16:9:16:39 | badProt ... otocol) | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:23:9:23:43 | badProt ... scheme) | This check does not consider vbscript:. | @@ -12,10 +11,5 @@ | IncompleteUrlSchemeCheck.js:87:7:87:40 | /^(java ... scheme) | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:94:10:94:15 | scheme | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:104:6:104:39 | /^(java ... scheme) | This check does not consider vbscript:. | -| IncompleteUrlSchemeCheck.js:110:12:112:29 | url // ... :/, "") | This check does not consider vbscript:. | +| IncompleteUrlSchemeCheck.js:110:12:112:29 | url\\n ... :/, "") | This check does not consider vbscript:. | | IncompleteUrlSchemeCheck.js:124:11:124:34 | url.rep ... :/, "") | This check does not consider vbscript:. | -testFailures -| IncompleteUrlSchemeCheck.js:94:10:94:15 | This check does not consider vbscript:. | Unexpected result: Alert | -| IncompleteUrlSchemeCheck.js:95:25:95:34 | // $ Alert | Missing result: Alert | -| IncompleteUrlSchemeCheck.js:110:12:112:29 | This check does not consider vbscript:. | Unexpected result: Alert | -| IncompleteUrlSchemeCheck.js:110:17:110:26 | // $ Alert | Missing result: Alert | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js index 65dfb679edc9..a4c6ed190f8f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js @@ -91,8 +91,8 @@ function test12(url) { function test13(url) { let scheme = goog.uri.utils.getScheme(url); - switch (scheme) { - case "javascript": // $ Alert + switch (scheme) { // $ Alert + case "javascript": case "data": return "about:blank"; default: @@ -107,9 +107,9 @@ function test14(url) { } function chain1(url) { - return url // $ Alert + return url .replace(/javascript:/, "") - .replace(/data:/, ""); + .replace(/data:/, ""); // $ Alert } function chain2(url) { From c2b65b1f854ee547ecac1f4c14744d6cf421f6b0 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 14:14:31 +0100 Subject: [PATCH 7/9] JS: Port IncompleteUrlSubstringSanitization test --- ...ncompleteUrlSubstringSanitization.expected | 5 ++ .../IncompleteUrlSubstringSanitization.qlref | 3 +- .../tst-IncompleteUrlSubstringSanitization.js | 60 +++++++++---------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected index fa1d5872ecb8..734b5db96a6d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected @@ -1,3 +1,4 @@ +problems | tst-IncompleteUrlSubstringSanitization.js:4:5:4:34 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:4:15:4:26 | "secure.com" | secure.com | | tst-IncompleteUrlSubstringSanitization.js:5:5:5:34 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:5:15:5:26 | "secure.net" | secure.net | | tst-IncompleteUrlSubstringSanitization.js:6:5:6:35 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:6:15:6:27 | ".secure.com" | .secure.com | @@ -23,3 +24,7 @@ | tst-IncompleteUrlSubstringSanitization.js:73:5:73:48 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:73:15:73:42 | "https: ... oo/bar" | https://secure.com/foo/bar | | tst-IncompleteUrlSubstringSanitization.js:74:5:74:40 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:74:15:74:34 | "https://secure.com" | https://secure.com | | tst-IncompleteUrlSubstringSanitization.js:75:5:75:52 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:75:15:75:46 | "https: ... ar-baz" | https://secure.com/foo/bar-baz | +testFailures +| tst-IncompleteUrlSubstringSanitization.js:62:2:62:31 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | Unexpected result: Alert | +| tst-IncompleteUrlSubstringSanitization.js:63:4:63:33 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | Unexpected result: Alert | +| tst-IncompleteUrlSubstringSanitization.js:64:3:64:26 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | Unexpected result: Alert | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.qlref index 3fa6794419d7..1c4c23821534 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.qlref +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.qlref @@ -1 +1,2 @@ -Security/CWE-020/IncompleteUrlSubstringSanitization.ql \ No newline at end of file +query: Security/CWE-020/IncompleteUrlSubstringSanitization.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js index efbaaff19865..9fc6dac602f6 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js @@ -1,23 +1,23 @@ (function(x){ - x.indexOf("internal") !== -1; // NOT OK, but not flagged - x.indexOf("localhost") !== -1; // NOT OK, but not flagged - x.indexOf("secure.com") !== -1; // NOT OK - x.indexOf("secure.net") !== -1; // NOT OK - x.indexOf(".secure.com") !== -1; // NOT OK - x.indexOf("sub.secure.") !== -1; // NOT OK, but not flagged - x.indexOf(".sub.secure.") !== -1; // NOT OK, but not flagged + x.indexOf("internal") !== -1; // $ MISSING: Alert + x.indexOf("localhost") !== -1; // $ MISSING: Alert + x.indexOf("secure.com") !== -1; // $ Alert + x.indexOf("secure.net") !== -1; // $ Alert + x.indexOf(".secure.com") !== -1; // $ Alert + x.indexOf("sub.secure.") !== -1; // $ MISSING: Alert + x.indexOf(".sub.secure.") !== -1; // $ MISSING: Alert - x.indexOf("secure.com") === -1; // NOT OK - x.indexOf("secure.com") === 0; // NOT OK - x.indexOf("secure.com") >= 0; // NOT OK + x.indexOf("secure.com") === -1; // $ Alert + x.indexOf("secure.com") === 0; // $ Alert + x.indexOf("secure.com") >= 0; // $ Alert - x.startsWith("https://secure.com"); // NOT OK - x.endsWith("secure.com"); // NOT OK + x.startsWith("https://secure.com"); // $ Alert + x.endsWith("secure.com"); // $ Alert x.endsWith(".secure.com"); // OK x.startsWith("secure.com/"); // OK x.indexOf("secure.com/") === 0; // OK - x.includes("secure.com"); // NOT OK + x.includes("secure.com"); // $ Alert x.indexOf("#") !== -1; // OK x.indexOf(":") !== -1; // OK @@ -29,11 +29,11 @@ x.indexOf("some/path") !== -1; // OK x.indexOf("/index.html") !== -1; // OK x.indexOf(":template:") !== -1; // OK - x.indexOf("https://secure.com") !== -1; // NOT OK - x.indexOf("https://secure.com:443") !== -1; // NOT OK - x.indexOf("https://secure.com/") !== -1; // NOT OK + x.indexOf("https://secure.com") !== -1; // $ Alert + x.indexOf("https://secure.com:443") !== -1; // $ Alert + x.indexOf("https://secure.com/") !== -1; // $ Alert - x.indexOf(".cn") !== -1; // NOT OK, but not flagged + x.indexOf(".cn") !== -1; // $ MISSING: Alert x.indexOf(".jpg") !== -1; // OK x.indexOf("index.html") !== -1; // OK x.indexOf("index.js") !== -1; // OK @@ -43,34 +43,34 @@ x.indexOf("secure=true") !== -1; // OK (query param) x.indexOf("&auth=") !== -1; // OK (query param) - x.indexOf(getCurrentDomain()) !== -1; // NOT OK, but not flagged - x.indexOf(location.origin) !== -1; // NOT OK, but not flagged + x.indexOf(getCurrentDomain()) !== -1; // $ MISSING: Alert + x.indexOf(location.origin) !== -1; // $ MISSING: Alert x.indexOf("tar.gz") + offset; // OK x.indexOf("tar.gz") - offset; // OK - x.indexOf("https://example.internal") !== -1; // NOT OK + x.indexOf("https://example.internal") !== -1; // $ Alert x.indexOf("https://") !== -1; // OK - x.startsWith("https://example.internal"); // NOT OK - x.indexOf('https://example.internal.org') !== 0; // NOT OK - x.indexOf('https://example.internal.org') === 0; // NOT OK - x.endsWith("internal.com"); // NOT OK + x.startsWith("https://example.internal"); // $ Alert + x.indexOf('https://example.internal.org') !== 0; // $ Alert + x.indexOf('https://example.internal.org') === 0; // $ Alert + x.endsWith("internal.com"); // $ Alert x.startsWith("https://example.internal:80"); // OK - x.indexOf("secure.com") !== -1; // NOT OK + x.indexOf("secure.com") !== -1; // $ Alert x.indexOf("secure.com") === -1; // OK !(x.indexOf("secure.com") !== -1); // OK !x.includes("secure.com"); // OK - if(!x.includes("secure.com")) { // NOT OK + if(!x.includes("secure.com")) { // $ Alert } else { doSomeThingWithTrustedURL(x); } - + x.startsWith("https://secure.com/foo/bar"); // OK - a forward slash after the domain makes prefix checks safe. - x.indexOf("https://secure.com/foo/bar") >= 0 // NOT OK - the url can be anywhere in the string. - x.indexOf("https://secure.com") >= 0 // NOT OK - x.indexOf("https://secure.com/foo/bar-baz") >= 0 // NOT OK - the url can be anywhere in the string. + x.indexOf("https://secure.com/foo/bar") >= 0 // $ Alert - the url can be anywhere in the string. + x.indexOf("https://secure.com") >= 0 // $ Alert + x.indexOf("https://secure.com/foo/bar-baz") >= 0 // $ Alert - the url can be anywhere in the string. }); From 18ab066e79940e1768192a540868b0d48e2fc31e Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 14:15:42 +0100 Subject: [PATCH 8/9] JS: Remove OK comments that don't provide further explanation --- .../tst-IncompleteUrlSubstringSanitization.js | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js index 9fc6dac602f6..de5b9e92d533 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js @@ -13,32 +13,32 @@ x.startsWith("https://secure.com"); // $ Alert x.endsWith("secure.com"); // $ Alert - x.endsWith(".secure.com"); // OK - x.startsWith("secure.com/"); // OK - x.indexOf("secure.com/") === 0; // OK + x.endsWith(".secure.com"); + x.startsWith("secure.com/"); + x.indexOf("secure.com/") === 0; x.includes("secure.com"); // $ Alert - x.indexOf("#") !== -1; // OK - x.indexOf(":") !== -1; // OK - x.indexOf(":/") !== -1; // OK - x.indexOf("://") !== -1; // OK - x.indexOf("//") !== -1; // OK - x.indexOf(":443") !== -1; // OK - x.indexOf("/some/path/") !== -1; // OK - x.indexOf("some/path") !== -1; // OK - x.indexOf("/index.html") !== -1; // OK - x.indexOf(":template:") !== -1; // OK + x.indexOf("#") !== -1; + x.indexOf(":") !== -1; + x.indexOf(":/") !== -1; + x.indexOf("://") !== -1; + x.indexOf("//") !== -1; + x.indexOf(":443") !== -1; + x.indexOf("/some/path/") !== -1; + x.indexOf("some/path") !== -1; + x.indexOf("/index.html") !== -1; + x.indexOf(":template:") !== -1; x.indexOf("https://secure.com") !== -1; // $ Alert x.indexOf("https://secure.com:443") !== -1; // $ Alert x.indexOf("https://secure.com/") !== -1; // $ Alert x.indexOf(".cn") !== -1; // $ MISSING: Alert - x.indexOf(".jpg") !== -1; // OK - x.indexOf("index.html") !== -1; // OK - x.indexOf("index.js") !== -1; // OK - x.indexOf("index.php") !== -1; // OK - x.indexOf("index.css") !== -1; // OK + x.indexOf(".jpg") !== -1; + x.indexOf("index.html") !== -1; + x.indexOf("index.js") !== -1; + x.indexOf("index.php") !== -1; + x.indexOf("index.css") !== -1; x.indexOf("secure=true") !== -1; // OK (query param) x.indexOf("&auth=") !== -1; // OK (query param) @@ -46,22 +46,22 @@ x.indexOf(getCurrentDomain()) !== -1; // $ MISSING: Alert x.indexOf(location.origin) !== -1; // $ MISSING: Alert - x.indexOf("tar.gz") + offset; // OK - x.indexOf("tar.gz") - offset; // OK + x.indexOf("tar.gz") + offset; + x.indexOf("tar.gz") - offset; x.indexOf("https://example.internal") !== -1; // $ Alert - x.indexOf("https://") !== -1; // OK + x.indexOf("https://") !== -1; x.startsWith("https://example.internal"); // $ Alert x.indexOf('https://example.internal.org') !== 0; // $ Alert x.indexOf('https://example.internal.org') === 0; // $ Alert x.endsWith("internal.com"); // $ Alert - x.startsWith("https://example.internal:80"); // OK + x.startsWith("https://example.internal:80"); x.indexOf("secure.com") !== -1; // $ Alert - x.indexOf("secure.com") === -1; // OK - !(x.indexOf("secure.com") !== -1); // OK - !x.includes("secure.com"); // OK + x.indexOf("secure.com") === -1; + !(x.indexOf("secure.com") !== -1); + !x.includes("secure.com"); if(!x.includes("secure.com")) { // $ Alert From bc34a045d3ceeaadf7967d4ae5c2e2086744ec8a Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 10 Jan 2025 14:17:34 +0100 Subject: [PATCH 9/9] JS: Triage discrepancies and update test --- .../IncompleteUrlSubstringSanitization.expected | 5 ----- .../tst-IncompleteUrlSubstringSanitization.js | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected index 734b5db96a6d..fa1d5872ecb8 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected @@ -1,4 +1,3 @@ -problems | tst-IncompleteUrlSubstringSanitization.js:4:5:4:34 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:4:15:4:26 | "secure.com" | secure.com | | tst-IncompleteUrlSubstringSanitization.js:5:5:5:34 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:5:15:5:26 | "secure.net" | secure.net | | tst-IncompleteUrlSubstringSanitization.js:6:5:6:35 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:6:15:6:27 | ".secure.com" | .secure.com | @@ -24,7 +23,3 @@ problems | tst-IncompleteUrlSubstringSanitization.js:73:5:73:48 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:73:15:73:42 | "https: ... oo/bar" | https://secure.com/foo/bar | | tst-IncompleteUrlSubstringSanitization.js:74:5:74:40 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:74:15:74:34 | "https://secure.com" | https://secure.com | | tst-IncompleteUrlSubstringSanitization.js:75:5:75:52 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:75:15:75:46 | "https: ... ar-baz" | https://secure.com/foo/bar-baz | -testFailures -| tst-IncompleteUrlSubstringSanitization.js:62:2:62:31 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | Unexpected result: Alert | -| tst-IncompleteUrlSubstringSanitization.js:63:4:63:33 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | Unexpected result: Alert | -| tst-IncompleteUrlSubstringSanitization.js:64:3:64:26 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | Unexpected result: Alert | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js index de5b9e92d533..f719a0835a6d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js @@ -59,9 +59,9 @@ x.startsWith("https://example.internal:80"); x.indexOf("secure.com") !== -1; // $ Alert - x.indexOf("secure.com") === -1; - !(x.indexOf("secure.com") !== -1); - !x.includes("secure.com"); + x.indexOf("secure.com") === -1; // $ Alert + !(x.indexOf("secure.com") !== -1); // $ Alert + !x.includes("secure.com"); // $ Alert if(!x.includes("secure.com")) { // $ Alert