From f79f342b20779812e52849674ffafbf084f6e75e Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Thu, 28 Mar 2019 22:06:46 +0100 Subject: [PATCH 1/4] JS: introduce SmallStrings.qll --- .../ql/src/Security/CWE-020/SmallStrings.qll | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 javascript/ql/src/Security/CWE-020/SmallStrings.qll diff --git a/javascript/ql/src/Security/CWE-020/SmallStrings.qll b/javascript/ql/src/Security/CWE-020/SmallStrings.qll new file mode 100644 index 000000000000..e02632b2f461 --- /dev/null +++ b/javascript/ql/src/Security/CWE-020/SmallStrings.qll @@ -0,0 +1,51 @@ +/** + * Provides predicates for restricting reasoning to small strings, in the name of performance. + */ + +import javascript + +/** + * Holds if `n` represents a small regular expression pattern, + * either because `n` is a regular expression literal, or because `n` is + * a string that could be used to construct a regular expression. + */ +pragma[noinline] +predicate isSmallRegExpPattern(DataFlow::Node n, string pattern) { + pattern.length() < 1000 and + ( + n.asExpr().(RegExpLiteral).getValue() = pattern + or + n.asExpr().(SimpleConstantString).getSimpleStringValue() = pattern + ) +} + +/** + * Holds if `n` represents a small string. + */ +pragma[noinline] +predicate isSmallString(DataFlow::Node n, string str) { + str.length() < 1000 and + n.asExpr().(SimpleConstantString).getSimpleStringValue() = str +} + +/** + * An expression that evaluates to a constant string in one step. + */ +private class SimpleConstantString extends ConstantString { + string str; + + SimpleConstantString() { + str = this.(StringLiteral).getValue() + or + exists(TemplateLiteral template | this = template | + template.getNumChildExpr() = 0 and + str = "" + or + template.getNumChildExpr() = 1 and + str = template.getElement(0).(TemplateElement).getValue() + ) + } + + /** Gets the constant string value this expression evaluates to. */ + string getSimpleStringValue() { result = str } +} From 6a42a6cf66cb2a6f95a6c87bfdaf909160136220 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Thu, 14 Mar 2019 08:02:29 +0100 Subject: [PATCH 2/4] JS: make RegExpPatterns::commonTLD more robust --- .../ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql | 4 ++-- .../Security/CWE-020/IncompleteUrlSubstringSanitization.ql | 4 ++-- javascript/ql/src/semmle/javascript/Regexp.qll | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql index 049b7f4c64a7..16dd4ae67fc3 100644 --- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql +++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql @@ -36,7 +36,7 @@ predicate isIncompleteHostNameRegExpPattern(string pattern, string hostPart) { // an unescaped single `.` "(?> - result = "com|org|edu|gov|uk|net|io" + result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])" } } From c15fc7a36b0bfdeba586df1c000b47c1d048f551 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Tue, 12 Mar 2019 13:35:38 +0100 Subject: [PATCH 3/4] JS: optimize and fixup js/incomplete-hostname-regexp The controversial bit of this is the change from `source.getStringValue()` to `source.asExpr().(SimpleConstantString).getSimpleStringValue()`. --- .../CWE-020/IncompleteHostnameRegExp.qhelp | 4 ++-- .../CWE-020/IncompleteHostnameRegExp.ql | 20 ++++++++++++++----- .../examples/IncompleteHostnameRegExp.js | 2 +- .../CWE-020/IncompleteHostnameRegExp.expected | 6 +++--- .../CWE-020/tst-IncompleteHostnameRegExp.js | 7 +++++-- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp index 71fefb488fa3..b770c061a5c2 100644 --- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp +++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp @@ -59,14 +59,14 @@

Address this vulnerability by escaping . - appropriately: let regex = /(www|beta|)\.example\.com/. + appropriately: let regex = /((www|beta)\.)?example\.com/.

-
  • OWASP: SSRF
    • +
  • MDN: Regular Expressions
  • OWASP: SSRF
  • OWASP: XSS Unvalidated Redirects and Forwards Cheat Sheet.
    •  diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql index 16dd4ae67fc3..3ee80ef88cc3 100644 --- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql +++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql @@ -11,18 +11,27 @@ */ import javascript +import SmallStrings /** * A taint tracking configuration for incomplete hostname regular expressions sources. */ -class Configuration extends TaintTracking::Configuration { +class Configuration extends DataFlow::Configuration { Configuration() { this = "IncompleteHostnameRegExpTracking" } override predicate isSource(DataFlow::Node source) { - isIncompleteHostNameRegExpPattern(source.getStringValue(), _) + exists (string pattern | + isSmallRegExpPattern(source, pattern) and + isIncompleteHostNameRegExpPattern(pattern, _) + ) } override predicate isSink(DataFlow::Node sink) { isInterpretedAsRegExp(sink) } + + override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + any(TaintTracking::AdditionalTaintStep dts).step(pred, succ) + } + } /** @@ -39,13 +48,14 @@ predicate isIncompleteHostNameRegExpPattern(string pattern, string hostPart) { "([():|?a-z0-9-]+(\\\\)?[.]" + RegExpPatterns::commonTLD() + ")" + ".*", 1) } -from Expr e, string pattern, string hostPart +from DataFlow::Node e, string pattern, string hostPart where + isSmallRegExpPattern(e, pattern) and ( - e.(RegExpLiteral).getValue() = pattern + e.asExpr() instanceof RegExpLiteral or exists(Configuration cfg | - cfg.hasFlow(e.flow(), _) and + cfg.hasFlow(e, _) and e.mayHaveStringValue(pattern) ) ) and diff --git a/javascript/ql/src/Security/CWE-020/examples/IncompleteHostnameRegExp.js b/javascript/ql/src/Security/CWE-020/examples/IncompleteHostnameRegExp.js index cc73a8388f6f..d3dfc3a867d0 100644 --- a/javascript/ql/src/Security/CWE-020/examples/IncompleteHostnameRegExp.js +++ b/javascript/ql/src/Security/CWE-020/examples/IncompleteHostnameRegExp.js @@ -2,7 +2,7 @@ app.get('/some/path', function(req, res) { let url = req.param('url'), host = urlLib.parse(url).host; // BAD: the host of `url` may be controlled by an attacker - let regex = /(www|beta|).example.com/; + let regex = /((www|beta).)?example.com/; if (host.match(regex)) { res.redirect(url); } diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected index cbb655c77bab..40a5f2852575 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected @@ -2,20 +2,20 @@ | tst-IncompleteHostnameRegExp.js:5:2:5:28 | /http:\\ ... le.net/ | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:6:2:6:42 | /http:\\ ... 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:11:13:11:37 | "http:/ ... le.com" | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | -| tst-IncompleteHostnameRegExp.js:12:10:12:34 | "http:/ ... le.com" | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | +| tst-IncompleteHostnameRegExp.js:12:10:12:35 | "^http: ... le.com" | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:15:22:15:46 | "http:/ ... le.com" | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:17:13:17:31 | `test.example.com$` | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | -| tst-IncompleteHostnameRegExp.js:17:14:17:30 | test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:19:17:19:34 | 'test.example.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:22:27:22:44 | 'test.example.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:28:22:28:39 | 'test.example.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | +| tst-IncompleteHostnameRegExp.js:30:30:30:47 | 'test.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 ... =$\|\\/)/ | 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\\// | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:39:2:39:34 | /\\(http ... m\\/\\)/g | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:40:2:40:29 | /https? ... le.com/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | -| tst-IncompleteHostnameRegExp.js:41:13:41:68 | '^http: ... e\\.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:41:41:41:68 | '^https ... e\\.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:42:13:42:61 | 'http[s ... \\/(.+)' | 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$/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:44:9:44:100 | 'protos ... ernal)' | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | | tst-IncompleteHostnameRegExp.js:46:2:46:26 | /exampl ... le.com/ | This regular expression has an unescaped '.' before 'dev\|example.com', so it might match more hosts than expected. | +| tst-IncompleteHostnameRegExp.js:48:41:48:68 | '^https ... e\\.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteHostnameRegExp.js index 45e6476a22e1..8407cece53c2 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteHostnameRegExp.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteHostnameRegExp.js @@ -9,7 +9,7 @@ /http:\/\/(?:.+)\\.test\\.example.com/; // NOT OK, but not yet supported with enough precision /http:\/\/test.example.com\/(?:.*)/; // OK new RegExp("http://test.example.com"); // NOT OK - s.match("http://test.example.com"); // NOT OK + s.match("^http://test.example.com"); // NOT OK function id(e) { return e; } new RegExp(id(id(id("http://test.example.com")))); // NOT OK @@ -27,7 +27,7 @@ } convert({ hostname: 'test.example.com' }); // NOT OK - let domains = [ { hostname: 'test.example.com' } ]; // NOT OK, but not yet supported + let domains = [ { hostname: 'test.example.com' } ]; // NOT OK function convert(domain) { return new RegExp(domain.hostname); } @@ -44,4 +44,7 @@ RegExp('protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); // NOT OK /example.dev|example.com/; // OK, but still flagged + + new RegExp('^http://localhost:8000|' + '^https?://.+\.example\.com'); // NOT OK + }); From 49fd7a12798f8f172fa735b53986bda17107da0d Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Tue, 12 Mar 2019 13:26:24 +0100 Subject: [PATCH 4/4] JS: reformulate js/incomplete-url-substring-sanitization - use taint tracking - restrict string sources to SimpleConstantString - turn into a path problem --- .../IncompleteUrlSubstringSanitization.ql | 113 +++++++++++++----- ...ncompleteUrlSubstringSanitization.expected | 89 ++++++++++---- .../tst-IncompleteUrlSubstringSanitization.js | 17 +++ 3 files changed, 165 insertions(+), 54 deletions(-) diff --git a/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql b/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql index 141158536404..f201ab7f19aa 100644 --- a/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql +++ b/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql @@ -1,7 +1,7 @@ /** * @name Incomplete URL substring sanitization * @description Security checks on the substrings of an unparsed URL are often vulnerable to bypassing. - * @kind problem + * @kind path-problem * @problem.severity warning * @precision high * @id js/incomplete-url-substring-sanitization @@ -11,7 +11,34 @@ */ import javascript -private import semmle.javascript.dataflow.InferredTypes +import DataFlow::PathGraph +import SmallStrings + +/** + * A node for a string value that looks like a URL. Used as a source + * for incomplete URL substring checks. + */ +class UrlStringSource extends DataFlow::Node { + string str; + + UrlStringSource() { + isSmallString(this, str) and + ( + // contains a domain on a common TLD, and perhaps some other URL components + str + .regexpMatch("(?i)([a-z]*:?//)?\\.?([a-z0-9-]+\\.)+" + RegExpPatterns::commonTLD() + + "(:[0-9]+)?/?") + or + // is a HTTP URL to a domain on any TLD + str.regexpMatch("(?i)https?://([a-z0-9-]+\\.)+([a-z]+)(:[0-9]+)?/?") + ) + } + + /** + * Gets the string value of this node. + */ + string getString() { result = str } +} /** * A check on a string for whether it contains a given substring, possibly with restrictions on the location of the substring. @@ -31,35 +58,57 @@ class SomeSubstringCheck extends DataFlow::Node { DataFlow::Node getSubstring() { result = substring } } -from SomeSubstringCheck check, DataFlow::Node substring, string target, string msg -where - substring = check.getSubstring() and - substring.mayHaveStringValue(target) and - ( - // target contains a domain on a common TLD, and perhaps some other URL components - target - .regexpMatch("(?i)([a-z]*:?//)?\\.?([a-z0-9-]+\\.)+" + RegExpPatterns::commonTLD() + - "(:[0-9]+)?/?") - or - // target is a HTTP URL to a domain on any TLD - target.regexpMatch("(?i)https?://([a-z0-9-]+\\.)+([a-z]+)(:[0-9]+)?/?") - ) and - ( - if check instanceof StringOps::StartsWith - then msg = "may be followed by an arbitrary host name" - else - if check instanceof StringOps::EndsWith - then msg = "may be preceded by an arbitrary host name" - else msg = "can be anywhere in the URL, and arbitrary hosts may come before or after it" +/** + * A taint tracking configuration for incomplete URL substring checks. + */ +class DomainUrlStringSubstringCheckConfiguration extends DataFlow::Configuration { + DomainUrlStringSubstringCheckConfiguration() { + this = "DomainUrlStringSubstringCheckConfiguration" + } + + override predicate isSource(DataFlow::Node source) { source instanceof UrlStringSource } + + override predicate isSink(DataFlow::Node sink) { any(SomeSubstringCheck s).getSubstring() = sink } + + override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + any(TaintTracking::AdditionalTaintStep dts).step(pred, succ) + } + +} + +/** + * Holds if `source` flows to `sink`, indicating a use of `substring` + * in an incomplete URL substring checks, explained by `msg`. + */ +predicate isIncompleteSubstringCheck( + DataFlow::PathNode source, DataFlow::PathNode sink, string substring, string msg +) { + exists(DomainUrlStringSubstringCheckConfiguration cfg | + cfg.hasFlowPath(source, sink) and + substring = source.getNode().(UrlStringSource).getString() ) and - // whitelist - not ( - // the leading dot in a subdomain sequence makes the suffix-check safe (if it is performed on the host of the url) - check instanceof StringOps::EndsWith and - target.regexpMatch("(?i)\\.([a-z0-9-]+)(\\.[a-z0-9-]+)+") - or - // the trailing port or slash makes the prefix-check safe - check instanceof StringOps::StartsWith and - target.regexpMatch(".*(:[0-9]+|/)") + exists(SomeSubstringCheck check | check.getSubstring() = sink.getNode() | + ( + if check instanceof StringOps::StartsWith + then msg = "may be followed by an arbitrary host name" + else + if check instanceof StringOps::EndsWith + then msg = "may be preceded by an arbitrary host name" + else msg = "can be anywhere in the URL, and arbitrary hosts may come before or after it" + ) and + // whitelist + not ( + // the leading dot in a subdomain sequence makes the suffix-check safe (if it is performed on the host of the url) + check instanceof StringOps::EndsWith and + substring.regexpMatch("(?i)\\.([a-z0-9-]+)(\\.[a-z0-9-]+)+") + or + // the trailing port or slash makes the prefix-check safe + check instanceof StringOps::StartsWith and + substring.regexpMatch(".*(:[0-9]+|/)") + ) ) -select check, "'$@' " + msg + ".", substring, target +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, string sourceString, string msg +where isIncompleteSubstringCheck(source, sink, sourceString, msg) +select sink.getNode(), source, sink, "'$@' " + msg + ".", source.getNode(), sourceString diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected index efbaad5a6729..7474fced479c 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected @@ -1,22 +1,67 @@ -| 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 | -| tst-IncompleteUrlSubstringSanitization.js:10:5:10:34 | x.index ... === -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:10:15:10:26 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:11:5:11:33 | x.index ... ) === 0 | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:11:15:11:26 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:12:5:12:32 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:12:15:12:26 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:14:5:14:38 | x.start ... e.com") | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:14:18:14:37 | "https://secure.com" | https://secure.com | -| tst-IncompleteUrlSubstringSanitization.js:15:5:15:28 | x.endsW ... e.com") | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:15:16:15:27 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:20:5:20:28 | x.inclu ... e.com") | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:20:16:20:27 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:32:5:32:42 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:32:15:32:34 | "https://secure.com" | https://secure.com | -| tst-IncompleteUrlSubstringSanitization.js:33:5:33:46 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:33:15:33:38 | "https: ... om:443" | https://secure.com:443 | -| tst-IncompleteUrlSubstringSanitization.js:34:5:34:43 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:34:15:34:35 | "https: ... e.com/" | https://secure.com/ | -| tst-IncompleteUrlSubstringSanitization.js:52:5:52:48 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:52:15:52:40 | "https: ... ternal" | https://example.internal | -| tst-IncompleteUrlSubstringSanitization.js:55:5:55:44 | x.start ... ernal") | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:55:18:55:43 | "https: ... ternal" | https://example.internal | -| tst-IncompleteUrlSubstringSanitization.js:56:5:56:51 | x.index ... ) !== 0 | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:56:15:56:44 | 'https: ... al.org' | https://example.internal.org | -| tst-IncompleteUrlSubstringSanitization.js:57:5:57:51 | x.index ... ) === 0 | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:57:15:57:44 | 'https: ... al.org' | https://example.internal.org | -| tst-IncompleteUrlSubstringSanitization.js:58:5:58:30 | x.endsW ... l.com") | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:58:16:58:29 | "internal.com" | internal.com | -| tst-IncompleteUrlSubstringSanitization.js:61:2:61:31 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:61:12:61:23 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:62:2:62:31 | x.index ... === -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:62:12:62:23 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:63:4:63:33 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:64:3:64:26 | x.inclu ... e.com") | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | secure.com | -| tst-IncompleteUrlSubstringSanitization.js:66:6:66:29 | x.inclu ... e.com") | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | secure.com | +nodes +| tst-IncompleteUrlSubstringSanitization.js:4:15:4:26 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:5:15:5:26 | "secure.net" | +| tst-IncompleteUrlSubstringSanitization.js:6:15:6:27 | ".secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:10:15:10:26 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:11:15:11:26 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:12:15:12:26 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:14:18:14:37 | "https://secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:15:16:15:27 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:16:16:16:28 | ".secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:17:18:17:30 | "secure.com/" | +| tst-IncompleteUrlSubstringSanitization.js:18:15:18:27 | "secure.com/" | +| tst-IncompleteUrlSubstringSanitization.js:20:16:20:27 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:32:15:32:34 | "https://secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:33:15:33:38 | "https: ... om:443" | +| tst-IncompleteUrlSubstringSanitization.js:34:15:34:35 | "https: ... e.com/" | +| tst-IncompleteUrlSubstringSanitization.js:52:15:52:40 | "https: ... ternal" | +| tst-IncompleteUrlSubstringSanitization.js:55:18:55:43 | "https: ... ternal" | +| tst-IncompleteUrlSubstringSanitization.js:56:15:56:44 | 'https: ... al.org' | +| tst-IncompleteUrlSubstringSanitization.js:57:15:57:44 | 'https: ... al.org' | +| tst-IncompleteUrlSubstringSanitization.js:58:16:58:29 | "internal.com" | +| tst-IncompleteUrlSubstringSanitization.js:59:18:59:46 | "https: ... nal:80" | +| tst-IncompleteUrlSubstringSanitization.js:61:12:61:23 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:62:12:62:23 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | +| tst-IncompleteUrlSubstringSanitization.js:72:6:75:2 | trustedDomains | +| tst-IncompleteUrlSubstringSanitization.js:72:23:75:2 | [\\n\\t\\t"se ... com"\\n\\t] | +| tst-IncompleteUrlSubstringSanitization.js:73:3:73:15 | "secure1.com" | +| tst-IncompleteUrlSubstringSanitization.js:74:3:74:16 | ".secure2.com" | +| tst-IncompleteUrlSubstringSanitization.js:77:12:77:24 | trustedDomain | +| tst-IncompleteUrlSubstringSanitization.js:77:29:77:42 | trustedDomains | +| tst-IncompleteUrlSubstringSanitization.js:78:24:78:36 | trustedDomain | +edges +| tst-IncompleteUrlSubstringSanitization.js:72:6:75:2 | trustedDomains | tst-IncompleteUrlSubstringSanitization.js:76:2:76:1 | trustedDomains | +| tst-IncompleteUrlSubstringSanitization.js:72:6:75:2 | trustedDomains | tst-IncompleteUrlSubstringSanitization.js:77:29:77:42 | trustedDomains | +| tst-IncompleteUrlSubstringSanitization.js:72:23:75:2 | [\\n\\t\\t"se ... com"\\n\\t] | tst-IncompleteUrlSubstringSanitization.js:72:6:75:2 | trustedDomains | +| tst-IncompleteUrlSubstringSanitization.js:73:3:73:15 | "secure1.com" | tst-IncompleteUrlSubstringSanitization.js:72:23:75:2 | [\\n\\t\\t"se ... com"\\n\\t] | +| tst-IncompleteUrlSubstringSanitization.js:74:3:74:16 | ".secure2.com" | tst-IncompleteUrlSubstringSanitization.js:72:23:75:2 | [\\n\\t\\t"se ... com"\\n\\t] | +| tst-IncompleteUrlSubstringSanitization.js:76:2:76:1 | trustedDomains | tst-IncompleteUrlSubstringSanitization.js:77:29:77:42 | trustedDomains | +| tst-IncompleteUrlSubstringSanitization.js:77:12:77:24 | trustedDomain | tst-IncompleteUrlSubstringSanitization.js:78:24:78:36 | trustedDomain | +| tst-IncompleteUrlSubstringSanitization.js:77:29:77:42 | trustedDomains | tst-IncompleteUrlSubstringSanitization.js:77:12:77:24 | trustedDomain | +#select +| tst-IncompleteUrlSubstringSanitization.js:4:15:4:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:4:15:4:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:4:15:4:26 | "secure.com" | '$@' 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:15:5:26 | "secure.net" | tst-IncompleteUrlSubstringSanitization.js:5:15:5:26 | "secure.net" | tst-IncompleteUrlSubstringSanitization.js:5:15:5:26 | "secure.net" | '$@' 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:15:6:27 | ".secure.com" | tst-IncompleteUrlSubstringSanitization.js:6:15:6:27 | ".secure.com" | tst-IncompleteUrlSubstringSanitization.js:6:15:6:27 | ".secure.com" | '$@' 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 | +| tst-IncompleteUrlSubstringSanitization.js:10:15:10:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:10:15:10:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:10:15:10:26 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:10:15:10:26 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:11:15:11:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:11:15:11:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:11:15:11:26 | "secure.com" | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:11:15:11:26 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:12:15:12:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:12:15:12:26 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:12:15:12:26 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:12:15:12:26 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:14:18:14:37 | "https://secure.com" | tst-IncompleteUrlSubstringSanitization.js:14:18:14:37 | "https://secure.com" | tst-IncompleteUrlSubstringSanitization.js:14:18:14:37 | "https://secure.com" | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:14:18:14:37 | "https://secure.com" | https://secure.com | +| tst-IncompleteUrlSubstringSanitization.js:15:16:15:27 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:15:16:15:27 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:15:16:15:27 | "secure.com" | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:15:16:15:27 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:20:16:20:27 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:20:16:20:27 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:20:16:20:27 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:20:16:20:27 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:32:15:32:34 | "https://secure.com" | tst-IncompleteUrlSubstringSanitization.js:32:15:32:34 | "https://secure.com" | tst-IncompleteUrlSubstringSanitization.js:32:15:32:34 | "https://secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:32:15:32:34 | "https://secure.com" | https://secure.com | +| tst-IncompleteUrlSubstringSanitization.js:33:15:33:38 | "https: ... om:443" | tst-IncompleteUrlSubstringSanitization.js:33:15:33:38 | "https: ... om:443" | tst-IncompleteUrlSubstringSanitization.js:33:15:33:38 | "https: ... om:443" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:33:15:33:38 | "https: ... om:443" | https://secure.com:443 | +| tst-IncompleteUrlSubstringSanitization.js:34:15:34:35 | "https: ... e.com/" | tst-IncompleteUrlSubstringSanitization.js:34:15:34:35 | "https: ... e.com/" | tst-IncompleteUrlSubstringSanitization.js:34:15:34:35 | "https: ... e.com/" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:34:15:34:35 | "https: ... e.com/" | https://secure.com/ | +| tst-IncompleteUrlSubstringSanitization.js:52:15:52:40 | "https: ... ternal" | tst-IncompleteUrlSubstringSanitization.js:52:15:52:40 | "https: ... ternal" | tst-IncompleteUrlSubstringSanitization.js:52:15:52:40 | "https: ... ternal" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:52:15:52:40 | "https: ... ternal" | https://example.internal | +| tst-IncompleteUrlSubstringSanitization.js:55:18:55:43 | "https: ... ternal" | tst-IncompleteUrlSubstringSanitization.js:55:18:55:43 | "https: ... ternal" | tst-IncompleteUrlSubstringSanitization.js:55:18:55:43 | "https: ... ternal" | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:55:18:55:43 | "https: ... ternal" | https://example.internal | +| tst-IncompleteUrlSubstringSanitization.js:56:15:56:44 | 'https: ... al.org' | tst-IncompleteUrlSubstringSanitization.js:56:15:56:44 | 'https: ... al.org' | tst-IncompleteUrlSubstringSanitization.js:56:15:56:44 | 'https: ... al.org' | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:56:15:56:44 | 'https: ... al.org' | https://example.internal.org | +| tst-IncompleteUrlSubstringSanitization.js:57:15:57:44 | 'https: ... al.org' | tst-IncompleteUrlSubstringSanitization.js:57:15:57:44 | 'https: ... al.org' | tst-IncompleteUrlSubstringSanitization.js:57:15:57:44 | 'https: ... al.org' | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:57:15:57:44 | 'https: ... al.org' | https://example.internal.org | +| tst-IncompleteUrlSubstringSanitization.js:58:16:58:29 | "internal.com" | tst-IncompleteUrlSubstringSanitization.js:58:16:58:29 | "internal.com" | tst-IncompleteUrlSubstringSanitization.js:58:16:58:29 | "internal.com" | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:58:16:58:29 | "internal.com" | internal.com | +| tst-IncompleteUrlSubstringSanitization.js:61:12:61:23 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:61:12:61:23 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:61:12:61:23 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:61:12:61:23 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:62:12:62:23 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:62:12:62:23 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:62:12:62:23 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:62:12:62:23 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | secure.com | +| tst-IncompleteUrlSubstringSanitization.js:78:24:78:36 | trustedDomain | tst-IncompleteUrlSubstringSanitization.js:73:3:73:15 | "secure1.com" | tst-IncompleteUrlSubstringSanitization.js:78:24:78:36 | trustedDomain | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.js:73:3:73:15 | "secure1.com" | secure1.com | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js b/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js index f7246c2a401b..a26729c32aa4 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js @@ -68,4 +68,21 @@ } else { doSomeThingWithTrustedURL(x); } + + let trustedDomains = [ + "secure1.com", // NOT OK, referenced below + ".secure2.com" + ]; + function isTrustedDomain(domain) { + for (let trustedDomain of trustedDomains) { + if (domain.endsWith(trustedDomain)) return true; // NOT OK + } + return false; + } + + let trustedHosts = [ + "secure1.com" + ]; + trustedHosts.includes(host); // OK: `Array.prototype.includes` + });