From 602538d1c1024dc794cb002d9e501680c2d8fab9 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Mon, 7 Mar 2022 16:08:42 +0100
Subject: [PATCH 01/13] Ruby: add RegExpPatterns module

---
 .../ruby/security/performance/RegExpTreeView.qll    | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
index f49592f96aa..c70e7c0dd7a 100644
--- a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
+++ b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
@@ -59,6 +59,19 @@ module RegExpFlags {
   }
 }
 
+/**
+ * Provides regular expression patterns.
+ */
+module RegExpPatterns {
+  /**
+   * Gets a pattern that matches common top-level domain names in lower case.
+   */
+  string commonTLD() {
+    // according to ranking by http://google.com/search?q=site:.<<TLD>>
+    result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
+  }
+}
+
 /**
  * An element containing a regular expression term, that is, either
  * a string literal (parsed as a regular expression)

From 832c9c4b0bdaaabd79d6b2f02d99bcad31db4867 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Tue, 1 Mar 2022 12:22:07 +0100
Subject: [PATCH 02/13] Ruby: copy IncompleteHostnameRegExp files from
 JavaScript

---
 .../security/cwe-020/HostnameRegexpShared.qll | 109 ++++++++++++++++++
 .../cwe-020/IncompleteHostnameRegExp.qhelp    |  73 ++++++++++++
 .../cwe-020/IncompleteHostnameRegExp.ql       | 107 +++++++++++++++++
 .../examples/IncompleteHostnameRegExp.rb      |   9 ++
 .../IncompleteHostnameRegExp.expected         |  26 +++++
 .../IncompleteHostnameRegExp.qlref            |   1 +
 .../IncompleteHostnameRegExp/hosttest.rb      |  23 ++++
 .../tst-IncompleteHostnameRegExp.rb           |  60 ++++++++++
 8 files changed, 408 insertions(+)
 create mode 100644 ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
 create mode 100644 ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
 create mode 100644 ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
 create mode 100644 ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb
 create mode 100644 ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
 create mode 100644 ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
 create mode 100644 ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb
 create mode 100644 ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb

diff --git a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
new file mode 100644
index 00000000000..97d75d4a524
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
@@ -0,0 +1,109 @@
+/**
+ * Provides predicates for reasoning about regular expressions
+ * that match URLs and hostname patterns.
+ */
+
+import javascript
+
+/**
+ * Holds if the given constant is unlikely to occur in the origin part of a URL.
+ */
+predicate isConstantInvalidInsideOrigin(RegExpConstant term) {
+  // Look for any of these cases:
+  // - A character that can't occur in the origin
+  // - Two dashes in a row
+  // - A colon that is not part of port or scheme separator
+  // - A slash that is not part of scheme separator
+  term.getValue().regexpMatch(".*(?:[^a-zA-Z0-9.:/-]|--|:[^0-9/]|(?<![/:]|^)/).*")
+}
+
+/** Holds if `term` is a dot constant of form `\.` or `[.]`. */
+predicate isDotConstant(RegExpTerm term) {
+  term.(RegExpCharEscape).getValue() = "."
+  or
+  exists(RegExpCharacterClass cls |
+    term = cls and
+    not cls.isInverted() and
+    cls.getNumChild() = 1 and
+    cls.getAChild().(RegExpConstant).getValue() = "."
+  )
+}
+
+/** Holds if `term` is a wildcard `.` or an actual `.` character. */
+predicate isDotLike(RegExpTerm term) {
+  term instanceof RegExpDot
+  or
+  isDotConstant(term)
+}
+
+/** Holds if `term` will only ever be matched against the beginning of the input. */
+predicate matchesBeginningOfString(RegExpTerm term) {
+  term.isRootTerm()
+  or
+  exists(RegExpTerm parent | matchesBeginningOfString(parent) |
+    term = parent.(RegExpSequence).getChild(0)
+    or
+    parent.(RegExpSequence).getChild(0) instanceof RegExpCaret and
+    term = parent.(RegExpSequence).getChild(1)
+    or
+    term = parent.(RegExpAlt).getAChild()
+    or
+    term = parent.(RegExpGroup).getAChild()
+  )
+}
+
+/**
+ * Holds if the given sequence contains top-level domain preceded by a dot, such as `.com`,
+ * excluding cases where this is at the very beginning of the regexp.
+ *
+ * `i` is bound to the index of the last child in the top-level domain part.
+ */
+predicate hasTopLevelDomainEnding(RegExpSequence seq, int i) {
+  seq.getChild(i)
+      .(RegExpConstant)
+      .getValue()
+      .regexpMatch("(?i)" + RegExpPatterns::commonTLD() + "(:\\d+)?([/?#].*)?") and
+  isDotLike(seq.getChild(i - 1)) and
+  not (i = 1 and matchesBeginningOfString(seq))
+}
+
+/**
+ * Holds if the given regular expression term contains top-level domain preceded by a dot,
+ * such as `.com`.
+ */
+predicate hasTopLevelDomainEnding(RegExpSequence seq) { hasTopLevelDomainEnding(seq, _) }
+
+/**
+ * Holds if `term` will always match a hostname, that is, all disjunctions contain
+ * a hostname pattern that isn't inside a quantifier.
+ */
+predicate alwaysMatchesHostname(RegExpTerm term) {
+  hasTopLevelDomainEnding(term, _)
+  or
+  // `localhost` is considered a hostname pattern, but has no TLD
+  term.(RegExpConstant).getValue().regexpMatch("\\blocalhost\\b")
+  or
+  not term instanceof RegExpAlt and
+  not term instanceof RegExpQuantifier and
+  alwaysMatchesHostname(term.getAChild())
+  or
+  alwaysMatchesHostnameAlt(term)
+}
+
+/** Holds if every child of `alt` contains a hostname pattern. */
+predicate alwaysMatchesHostnameAlt(RegExpAlt alt) {
+  alwaysMatchesHostnameAlt(alt, alt.getNumChild() - 1)
+}
+
+/**
+ * Holds if the first `i` children of `alt` contains a hostname pattern.
+ *
+ * This is used instead of `forall` to avoid materializing the set of alternatives
+ * that don't contains hostnames, which is much larger.
+ */
+predicate alwaysMatchesHostnameAlt(RegExpAlt alt, int i) {
+  alwaysMatchesHostname(alt.getChild(0)) and i = 0
+  or
+  alwaysMatchesHostnameAlt(alt, i - 1) and
+  alwaysMatchesHostname(alt.getChild(i))
+}
diff --git a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
new file mode 100644
index 00000000000..a5e0a78b8cc
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
@@ -0,0 +1,73 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+	<overview>
+		<p>
+
+			Sanitizing untrusted URLs is an important technique for
+			preventing attacks such as request forgeries and malicious
+			redirections. Often, this is done by checking that the host of a URL
+			is in a set of allowed hosts.
+
+		</p>
+
+		<p>
+
+			If a regular expression implements such a check, it is
+			easy to accidentally make the check too permissive by not escaping the
+			<code>.</code> meta-characters appropriately.
+
+			Even if the check is not used in a security-critical
+			context, the incomplete check may still cause undesirable behaviors
+			when it accidentally succeeds.
+
+		</p>
+	</overview>
+
+	<recommendation>
+		<p>
+
+			Escape all meta-characters appropriately when constructing
+			regular expressions for security checks, pay special attention to the
+			<code>.</code> meta-character.
+
+		</p>
+	</recommendation>
+
+	<example>
+
+		<p>
+
+			The following example code checks that a URL redirection
+			will reach the <code>example.com</code> domain, or one of its
+			subdomains.
+
+		</p>
+
+		<sample src="examples/IncompleteHostnameRegExp.js"/>
+
+		<p>
+
+			The check is however easy to bypass because the unescaped
+			<code>.</code> allows for any character before
+			<code>example.com</code>, effectively allowing the redirect to go to
+			an attacker-controlled domain such as <code>wwwXexample.com</code>.
+
+		</p>
+		<p>
+
+			Address this vulnerability by escaping <code>.</code>
+			appropriately: <code>let regex = /((www|beta)\.)?example\.com/</code>.
+
+		</p>
+
+	</example>
+
+	<references>
+		<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions">Regular Expressions</a></li>
+		<li>OWASP: <a href="https://www.owasp.org/index.php/Server_Side_Request_Forgery">SSRF</a></li>
+		<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html">XSS Unvalidated Redirects and Forwards Cheat Sheet</a>.</li>
+	</references>
+</qhelp>
diff --git a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
new file mode 100644
index 00000000000..bd095887011
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
@@ -0,0 +1,107 @@
+/**
+ * @name Incomplete regular expression for hostnames
+ * @description Matching a URL or hostname against a regular expression that contains an unescaped dot as part of the hostname might match more hostnames than expected.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 7.8
+ * @precision high
+ * @id js/incomplete-hostname-regexp
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-020
+ */
+
+import javascript
+import semmle.javascript.CharacterEscapes
+import HostnameRegexpShared
+
+/**
+ * Holds if `term` occurs inside a quantifier or alternative (and thus
+ * can not be expected to correspond to a unique match), or as part of
+ * a lookaround assertion (which are rarely used for capture groups).
+ */
+predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
+  exists(RegExpParent parent | parent = term.getParent() |
+    parent instanceof RegExpAlt
+    or
+    parent instanceof RegExpQuantifier
+    or
+    parent instanceof RegExpSubPattern
+    or
+    isInsideChoiceOrSubPattern(parent)
+  )
+}
+
+/**
+ * Holds if `group` is likely to be used as a capture group.
+ */
+predicate isLikelyCaptureGroup(RegExpGroup group) {
+  group.isCapture() and
+  not isInsideChoiceOrSubPattern(group)
+}
+
+/**
+ * Holds if `seq` contains two consecutive dots `..` or escaped dots.
+ *
+ * At least one of these dots is not intended to be a subdomain separator,
+ * so we avoid flagging the pattern in this case.
+ */
+predicate hasConsecutiveDots(RegExpSequence seq) {
+  exists(int i |
+    isDotLike(seq.getChild(i)) and
+    isDotLike(seq.getChild(i + 1))
+  )
+}
+
+predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
+  seq = regexp.getAChild*() and
+  exists(RegExpDot unescapedDot, int i, string hostname |
+    hasTopLevelDomainEnding(seq, i) and
+    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
+    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
+    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
+    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
+    not hasConsecutiveDots(unescapedDot.getParent()) and
+    hostname =
+      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
+        seq.getChild(i).getRawValue()
+  |
+    if unescapedDot.getParent() instanceof RegExpQuantifier
+    then
+      // `.*\.example.com` can match `evil.com/?x=.example.com`
+      //
+      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
+      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
+      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
+      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
+      seq.getChild(0) instanceof RegExpCaret and
+      not seq.getAChild() instanceof RegExpDollar and
+      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
+      msg =
+        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
+          + "' which may cause '" + hostname +
+          "' to be matched anywhere in the URL, outside the hostname."
+    else
+      msg =
+        "has an unescaped '.' before '" + hostname +
+          "', so it might match more hosts than expected."
+  )
+}
+
+from
+  RegExpPatternSource re, RegExpTerm regexp, RegExpSequence hostSequence, string msg, string kind,
+  DataFlow::Node aux
+where
+  regexp = re.getRegExpTerm() and
+  isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
+  (
+    if re.getAParse() != re
+    then (
+      kind = "string, which is used as a regular expression $@," and
+      aux = re.getAParse()
+    ) else (
+      kind = "regular expression" and aux = re
+    )
+  ) and
+  not CharacterEscapes::hasALikelyRegExpPatternMistake(re)
+select hostSequence, "This " + kind + " " + msg, aux, "here"
diff --git a/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb b/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb
new file mode 100644
index 00000000000..11d11a97ca4
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb
@@ -0,0 +1,9 @@
+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/;
+    if (host.match(regex)) {
+        res.redirect(url);
+    }
+});
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
new file mode 100644
index 00000000000..13950be73ba
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
@@ -0,0 +1,26 @@
+| 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: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 |
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
new file mode 100644
index 00000000000..e818d947252
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
@@ -0,0 +1 @@
+Security/CWE-020/IncompleteHostnameRegExp.ql
\ No newline at end of file
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb
new file mode 100644
index 00000000000..ca7f7094a8c
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb
@@ -0,0 +1,23 @@
+from flask import Flask, request, redirect
+import re
+
+app = Flask(__name__)
+
+UNSAFE_REGEX = re.compile("(www|beta).example.com/")
+SAFE_REGEX = re.compile(r"(www|beta)\.example\.com/")
+
+@app.route('/some/path/bad')
+def unsafe(request):
+    target = request.args.get('target', '')
+    if UNSAFE_REGEX.match(target):
+        return redirect(target)
+
+@app.route('/some/path/good')
+def safe(request):
+    target = request.args.get('target', '')
+    if SAFE_REGEX.match(target):
+        return redirect(target)
+
+# FP reported in https://github.com/github/codeql/issues/3712
+# This does not define a regex (but could be used by other code to do so)
+escaped = re.escape("https://www.humblebundle.com/home/library")
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb
new file mode 100644
index 00000000000..df887092d23
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb
@@ -0,0 +1,60 @@
+(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
+
+	function id(e) { return e; }
+	new RegExp(id(id(id("^http://test.example.com")))); // NOT OK
+
+	new RegExp(`test.example.com$`); // NOT OK
+
+	let hostname = '^test.example.com'; // NOT OK
+	new RegExp(`${hostname}$`);
+
+	let domain = { hostname: 'test.example.com$' }; // NOT OK
+	new RegExp(domain.hostname);
+
+	function convert1(domain) {
+		return new RegExp(domain.hostname);
+	}
+	convert1({ hostname: 'test.example.com$' }); // NOT OK
+
+	let domains = [ { hostname: 'test.example.com$' } ];  // NOT OK
+	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.dev|example.com)/; // OK
+
+	new RegExp('^http://localhost:8000|' + '^https?://.+.example\\.com/'); // NOT OK
+
+	var primary = 'example.com$';
+	new RegExp('test.' + primary); // NOT OK, but not detected
+
+	new RegExp('test.' + 'example.com$'); // NOT OK
+
+	new RegExp('^http://test\.example.com'); // NOT OK, but flagged by js/useless-regexp-character-escape
+
+	/^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
+});

From 9e8930c19278ca9fe4ef77115f32c6ace3a0f17b Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 9 Feb 2022 18:07:14 +0100
Subject: [PATCH 03/13] Ruby: IncompleteHostnameRegExp.ql

---
 .../CWE-020/IncompleteHostnameRegExp.qhelp    |   2 +-
 .../CWE-020/IncompleteHostnameRegExp.qhelp    |   2 +-
 .../security/performance/RegExpTreeView.qll   |   3 +
 .../2022-02-10-incomplete-hostname-regexp.md  |   4 +
 .../security/cwe-020/HostnameRegexpShared.qll |   5 +-
 .../cwe-020/IncompleteHostnameRegExp.qhelp    |   7 +-
 .../cwe-020/IncompleteHostnameRegExp.ql       |  10 +-
 .../examples/IncompleteHostnameRegExp.rb      |  22 ++--
 .../IncompleteHostnameRegExp.expected         |  58 +++++-----
 .../IncompleteHostnameRegExp.qlref            |   2 +-
 .../IncompleteHostnameRegExp/hosttest.rb      |  35 +++---
 .../tst-IncompleteHostnameRegExp.rb           | 101 ++++++++++--------
 12 files changed, 137 insertions(+), 114 deletions(-)
 create mode 100644 ruby/ql/src/change-notes/2022-02-10-incomplete-hostname-regexp.md

diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
index a5e0a78b8cc..86637d140df 100644
--- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
+++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
@@ -30,7 +30,7 @@
 		<p>
 
 			Escape all meta-characters appropriately when constructing
-			regular expressions for security checks, pay special attention to the
+			regular expressions for security checks, and pay special attention to the
 			<code>.</code> meta-character.
 
 		</p>
diff --git a/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp b/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
index d112cbca508..66c9dd847c7 100644
--- a/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
+++ b/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
@@ -30,7 +30,7 @@
         <p>
 
             Escape all meta-characters appropriately when constructing
-            regular expressions for security checks, pay special attention to the
+            regular expressions for security checks, and pay special attention to the
             <code>.</code> meta-character.
 
         </p>
diff --git a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
index c70e7c0dd7a..9043fa949ba 100644
--- a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
+++ b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
@@ -637,6 +637,9 @@ class RegExpGroup extends RegExpTerm, TRegExpGroup {
    */
   int getNumber() { result = re.getGroupNumber(start, end) }
 
+  /** Holds if this is a capture group. */
+  predicate isCapture() { not exists(this.getNumber()) }
+
   /** Holds if this is a named capture group. */
   predicate isNamed() { exists(this.getName()) }
 
diff --git a/ruby/ql/src/change-notes/2022-02-10-incomplete-hostname-regexp.md b/ruby/ql/src/change-notes/2022-02-10-incomplete-hostname-regexp.md
new file mode 100644
index 00000000000..f87676dc188
--- /dev/null
+++ b/ruby/ql/src/change-notes/2022-02-10-incomplete-hostname-regexp.md
@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `rb/incomplete-hostname-regexp`. The query finds instances where a hostname is incompletely sanitized due to an unescaped character in a regular expression.
diff --git a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
index 97d75d4a524..102d719979e 100644
--- a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
+++ b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
@@ -3,7 +3,8 @@
  * that match URLs and hostname patterns.
  */
 
-import javascript
+private import codeql.ruby.security.performance.RegExpTreeView
+private import codeql.ruby.DataFlow
 
 /**
  * Holds if the given constant is unlikely to occur in the origin part of a URL.
@@ -19,7 +20,7 @@ predicate isConstantInvalidInsideOrigin(RegExpConstant term) {
 
 /** Holds if `term` is a dot constant of form `\.` or `[.]`. */
 predicate isDotConstant(RegExpTerm term) {
-  term.(RegExpCharEscape).getValue() = "."
+  term.(RegExpEscape).getValue() = "."
   or
   exists(RegExpCharacterClass cls |
     term = cls and
diff --git a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
index a5e0a78b8cc..cb242aa8c61 100644
--- a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
+++ b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
@@ -30,7 +30,7 @@
 		<p>
 
 			Escape all meta-characters appropriately when constructing
-			regular expressions for security checks, pay special attention to the
+			regular expressions for security checks, and pay special attention to the
 			<code>.</code> meta-character.
 
 		</p>
@@ -46,7 +46,7 @@
 
 		</p>
 
-		<sample src="examples/IncompleteHostnameRegExp.js"/>
+		<sample src="examples/IncompleteHostnameRegExp.rb"/>
 
 		<p>
 
@@ -59,14 +59,13 @@
 		<p>
 
 			Address this vulnerability by escaping <code>.</code>
-			appropriately: <code>let regex = /((www|beta)\.)?example\.com/</code>.
+			appropriately: <code>regex = /((www|beta)\.)?example\.com/</code>.
 
 		</p>
 
 	</example>
 
 	<references>
-		<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions">Regular Expressions</a></li>
 		<li>OWASP: <a href="https://www.owasp.org/index.php/Server_Side_Request_Forgery">SSRF</a></li>
 		<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html">XSS Unvalidated Redirects and Forwards Cheat Sheet</a>.</li>
 	</references>
diff --git a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
index bd095887011..5526978b07a 100644
--- a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
+++ b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
@@ -5,15 +5,16 @@
  * @problem.severity warning
  * @security-severity 7.8
  * @precision high
- * @id js/incomplete-hostname-regexp
+ * @id rb/incomplete-hostname-regexp
  * @tags correctness
  *       security
  *       external/cwe/cwe-020
  */
 
-import javascript
-import semmle.javascript.CharacterEscapes
 import HostnameRegexpShared
+import codeql.ruby.security.performance.RegExpTreeView
+import codeql.ruby.DataFlow
+import codeql.ruby.AST as AST
 
 /**
  * Holds if `term` occurs inside a quantifier or alternative (and thus
@@ -102,6 +103,5 @@ where
     ) else (
       kind = "regular expression" and aux = re
     )
-  ) and
-  not CharacterEscapes::hasALikelyRegExpPatternMistake(re)
+  )
 select hostSequence, "This " + kind + " " + msg, aux, "here"
diff --git a/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb b/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb
index 11d11a97ca4..b0a64a658dd 100644
--- a/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb
+++ b/ruby/ql/src/queries/security/cwe-020/examples/IncompleteHostnameRegExp.rb
@@ -1,9 +1,13 @@
-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/;
-    if (host.match(regex)) {
-        res.redirect(url);
-    }
-});
+class AppController < ApplicationController
+
+    def index
+        url = params[:url]
+        host = URI(url).host
+        # BAD: the host of `url` may be controlled by an attacker
+        regex = /^((www|beta).)?example.com/
+        if host.match(regex)
+            redirect_to url
+        end
+    end
+
+end
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
index 13950be73ba..944cb284f11 100644
--- a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
@@ -1,26 +1,32 @@
-| 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: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 |
+| hosttest.rb:1:18:1:41 | (www\|beta).example.com\\/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | hosttest.rb:1:17:1:42 | /(www\|beta).example.com\\// | here |
+| hosttest.rb:2:33:2:55 | (www\|beta).example.com/ | This regular expression has an unescaped '.' before 'example.com/', so it might match more hosts than expected. | hosttest.rb:2:32:2:56 | "(www\|beta).example.com/" | here |
+| hosttest.rb:3:29:3:51 | (www\|beta).example.com/ | This regular expression has an unescaped '.' before 'example.com/', so it might match more hosts than expected. | hosttest.rb:3:28:3:52 | "(www\|beta).example.com/" | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:3:2:3:29 | /^http:\\/\\/test.example.com/ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:5:2:5:29 | /^http:\\/\\/test.example.net/ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:6:2:6:43 | /^http:\\/\\/test.(example-a\|exa.../ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:7:2:7:31 | /^http:\\/\\/(.+).example.com\\// | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:7:2:7:31 | /^http:\\/\\/(.+).example.com\\// | here |
+| tst-IncompleteHostnameRegExp.rb:9:3:9: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.rb:9:2:9:40 | /^http:\\/\\/(?:.+)\\.test\\.examp.../ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:10:2:10:37 | /^http:\\/\\/test.example.com\\/(.../ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:11:13:11:38 | "^http://test.example.com" | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:12:14:12:39 | "^http://test.example.com" | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:15:13:15:50 | call to id | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:17:13:17:31 | `test.example.com$` | here |
+| tst-IncompleteHostnameRegExp.rb:19:14:19:30 | ^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.rb:20:13:20:26 | "#{...}$" | here |
+| tst-IncompleteHostnameRegExp.rb:20:14:20:31 | ^test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:20:13:20:26 | "#{...}$" | here |
+| tst-IncompleteHostnameRegExp.rb:22:24:22: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.rb:23:13:23:29 | ...[...] | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:63:20:63:36 | ...[...] | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:32:21:32:35 | domain.hostname | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:37:2:37:54 | /^(https?:)?\\/\\/((service\|www).../ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:38:2:38:44 | /^(http\|https):\\/\\/www.example.../ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:39:2:39:33 | /^(http:\\/\\/sub.example.com\\/)/ | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:40:2:40:30 | /^https?:\\/\\/api.example.com/ | here |
+| tst-IncompleteHostnameRegExp.rb:41:42:41:68 | ^https?://.+\\.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:41:13:41:71 | ... + ... | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:43:2:43:33 | /^https:\\/\\/[a-z]*.example.com$/ | here |
+| tst-IncompleteHostnameRegExp.rb:44:40:44:53 | .+.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
+| tst-IncompleteHostnameRegExp.rb:44:55:44:70 | .+.example-a.com | This regular expression has an unescaped '.' before 'example-a.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
+| tst-IncompleteHostnameRegExp.rb:44:72:44:87 | .+.example-b.com | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
+| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.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.rb:48:13:48:69 | ... + ... | here |
+| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:48:13:48:69 | ... + ... | here |
+| tst-IncompleteHostnameRegExp.rb:53:13:53:36 | test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:53:13:53:36 | ... + ... | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:59:2:59:32 | /^(foo.example\\.com\|whatever)$/ | here |
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
index e818d947252..7fd45d159ce 100644
--- a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
@@ -1 +1 @@
-Security/CWE-020/IncompleteHostnameRegExp.ql
\ No newline at end of file
+queries/security/cwe-020/IncompleteHostnameRegExp.ql
\ No newline at end of file
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb
index ca7f7094a8c..5a5c96692ce 100644
--- a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/hosttest.rb
@@ -1,23 +1,22 @@
-from flask import Flask, request, redirect
-import re
+UNSAFE_REGEX1 = /(www|beta).example.com\//
+UNSAFE_REGEX2 = Regexp.compile("(www|beta).example.com/")
+UNSAFE_REGEX3 = Regexp.new("(www|beta).example.com/")
+SAFE_REGEX = /(www|beta)\.example\.com\//
 
-app = Flask(__name__)
+def unsafe
+    target = params[:target]
+    if UNSAFE_REGEX1.match(target)
+        redirect_to target
+    end
+end
 
-UNSAFE_REGEX = re.compile("(www|beta).example.com/")
-SAFE_REGEX = re.compile(r"(www|beta)\.example\.com/")
-
-@app.route('/some/path/bad')
-def unsafe(request):
-    target = request.args.get('target', '')
-    if UNSAFE_REGEX.match(target):
-        return redirect(target)
-
-@app.route('/some/path/good')
-def safe(request):
-    target = request.args.get('target', '')
-    if SAFE_REGEX.match(target):
-        return redirect(target)
+def safe
+    target = params[:target]
+    if SAFE_REGEX.match(target)
+        redirect_to target
+    end
+end
 
 # FP reported in https://github.com/github/codeql/issues/3712
 # This does not define a regex (but could be used by other code to do so)
-escaped = re.escape("https://www.humblebundle.com/home/library")
+escaped = Regexp::escape("https://www.humblebundle.com/home/library")
diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb
index df887092d23..5d7f7afb133 100644
--- a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb
@@ -1,60 +1,67 @@
-(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
+def foo
+	/^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
+	Regexp.new("^http://test.example.com"); # NOT OK
+	if (s.match("^http://test.example.com")); end # NOT OK
 
-	function id(e) { return e; }
-	new RegExp(id(id(id("^http://test.example.com")))); // NOT OK
 
-	new RegExp(`test.example.com$`); // NOT OK
+	Regexp.new(id(id(id("^http://test.example.com")))); # NOT OK
 
-	let hostname = '^test.example.com'; // NOT OK
-	new RegExp(`${hostname}$`);
+	Regexp.new(`test.example.com$`); # NOT OK
 
-	let domain = { hostname: 'test.example.com$' }; // NOT OK
-	new RegExp(domain.hostname);
+	hostname = '^test.example.com'; # NOT OK
+	Regexp.new("#{hostname}$");
 
-	function convert1(domain) {
-		return new RegExp(domain.hostname);
-	}
-	convert1({ hostname: 'test.example.com$' }); // NOT OK
+	domain = { hostname: 'test.example.com$' }; # NOT OK
+	Regexp.new(domain[:hostname]);
 
-	let domains = [ { hostname: 'test.example.com$' } ];  // NOT OK
-	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.dev|example.com)/; // OK
 
-	new RegExp('^http://localhost:8000|' + '^https?://.+.example\\.com/'); // NOT OK
+	convert1({ hostname: 'test.example.com$' }); # NOT OK
 
-	var primary = 'example.com$';
-	new RegExp('test.' + primary); // NOT OK, but not detected
+	domains = [ { hostname: 'test.example.com$' } ];  # NOT OK
 
-	new RegExp('test.' + 'example.com$'); // NOT OK
 
-	new RegExp('^http://test\.example.com'); // NOT OK, but flagged by js/useless-regexp-character-escape
 
-	/^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
-});
+	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\/)/i; # NOT OK
+	/^https?:\/\/api.example.com/; # NOT OK
+	Regexp.new('^http://localhost:8000|' + "^https?://.+\\.example\\.com/"); # NOT OK
+	Regexp.new("^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)"); # NOT OK
+	/^https:\/\/[a-z]*.example.com$/; # NOT OK
+	Regexp.compile('^protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); # NOT OK
+
+	/^(example.dev|example.com)/; # OK
+
+	Regexp.new('^http://localhost:8000|' + "^https?://.+.example\\.com/"); # NOT OK
+
+	primary = 'example.com$';
+	Regexp.new('test.' + primary); # NOT OK, but not detected
+
+	Regexp.new('test.' + 'example.com$'); # NOT OK
+
+	Regexp.new('^http://test\.example.com'); # NOT OK
+
+	/^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
+end
+def id(e); return e; end
+def convert1(domain)
+	return Regexp.new(domain[:hostname]);
+end
+def convert2(domain)
+	return Regexp.new(domain[:hostname]);
+end

From 097c661362c317eb9bf15a009f045baeee5950bd Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Mon, 28 Feb 2022 19:07:28 +0100
Subject: [PATCH 04/13] Ruby: drop results that cannot be found yet from
 IncompleteHostnameRegExp.expected

---
 .../IncompleteHostnameRegExp.expected                     | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
index 944cb284f11..f70cbc19ce7 100644
--- a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
@@ -10,23 +10,15 @@
 | tst-IncompleteHostnameRegExp.rb: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.rb:10:2:10:37 | /^http:\\/\\/test.example.com\\/(.../ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:11:13:11:38 | "^http://test.example.com" | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:12:14:12:39 | "^http://test.example.com" | here |
-| tst-IncompleteHostnameRegExp.rb: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.rb:15:13:15:50 | call to id | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:17:13:17:31 | `test.example.com$` | here |
 | tst-IncompleteHostnameRegExp.rb:19:14:19:30 | ^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.rb:20:13:20:26 | "#{...}$" | here |
 | tst-IncompleteHostnameRegExp.rb:20:14:20:31 | ^test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:20:13:20:26 | "#{...}$" | here |
-| tst-IncompleteHostnameRegExp.rb:22:24:22: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.rb:23:13:23:29 | ...[...] | here |
-| tst-IncompleteHostnameRegExp.rb: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.rb:63:20:63:36 | ...[...] | here |
-| tst-IncompleteHostnameRegExp.rb: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.rb:32:21:32:35 | domain.hostname | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:37:2:37:54 | /^(https?:)?\\/\\/((service\|www).../ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:38:2:38:44 | /^(http\|https):\\/\\/www.example.../ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:39:2:39:33 | /^(http:\\/\\/sub.example.com\\/)/ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:40:2:40:30 | /^https?:\\/\\/api.example.com/ | here |
-| tst-IncompleteHostnameRegExp.rb:41:42:41:68 | ^https?://.+\\.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:41:13:41:71 | ... + ... | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:43:2:43:33 | /^https:\\/\\/[a-z]*.example.com$/ | here |
 | tst-IncompleteHostnameRegExp.rb:44:40:44:53 | .+.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
 | tst-IncompleteHostnameRegExp.rb:44:55:44:70 | .+.example-a.com | This regular expression has an unescaped '.' before 'example-a.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
 | tst-IncompleteHostnameRegExp.rb:44:72:44:87 | .+.example-b.com | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
-| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.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.rb:48:13:48:69 | ... + ... | here |
-| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:48:13:48:69 | ... + ... | here |
-| tst-IncompleteHostnameRegExp.rb:53:13:53:36 | test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:53:13:53:36 | ... + ... | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:59:2:59:32 | /^(foo.example\\.com\|whatever)$/ | here |

From 98f56f4d6015d431b8fe13c407beece7f42ccda7 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 2 Mar 2022 18:28:47 +0100
Subject: [PATCH 05/13] Js/Ruby: Share IncompleteHostnameRegExp.ql

---
 config/identical-files.json                   |  4 +
 .../Security/CWE-020/HostnameRegexpShared.qll | 95 +++++++++++++++++-
 .../CWE-020/HostnameRegexpSpecific.qll        |  1 +
 .../CWE-020/IncompleteHostnameRegExp.ql       | 93 +-----------------
 .../security/performance/RegExpTreeView.qll   |  2 +
 .../security/cwe-020/HostnameRegexpShared.qll | 98 ++++++++++++++++++-
 .../cwe-020/HostnameRegexpSpecific.qll        |  2 +
 .../cwe-020/IncompleteHostnameRegExp.ql       | 93 +-----------------
 8 files changed, 200 insertions(+), 188 deletions(-)
 create mode 100644 javascript/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
 create mode 100644 ruby/ql/src/queries/security/cwe-020/HostnameRegexpSpecific.qll

diff --git a/config/identical-files.json b/config/identical-files.json
index bef8b72c87e..a7db98e94eb 100644
--- a/config/identical-files.json
+++ b/config/identical-files.json
@@ -508,5 +508,9 @@
     "java/ql/lib/semmle/code/java/dataflow/internal/AccessPathSyntax.qll",
     "javascript/ql/lib/semmle/javascript/frameworks/data/internal/AccessPathSyntax.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/AccessPathSyntax.qll"
+  ],
+  "Hostname Regexp queries": [
+    "javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll",
+    "ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll"
   ]
 }
diff --git a/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll b/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll
index 97d75d4a524..d003d4b9a10 100644
--- a/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll
+++ b/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll
@@ -3,7 +3,7 @@
  * that match URLs and hostname patterns.
  */
 
-import javascript
+private import HostnameRegexpSpecific
 
 /**
  * Holds if the given constant is unlikely to occur in the origin part of a URL.
@@ -107,3 +107,96 @@ predicate alwaysMatchesHostnameAlt(RegExpAlt alt, int i) {
   alwaysMatchesHostnameAlt(alt, i - 1) and
   alwaysMatchesHostname(alt.getChild(i))
 }
+
+/**
+ * Holds if `term` occurs inside a quantifier or alternative (and thus
+ * can not be expected to correspond to a unique match), or as part of
+ * a lookaround assertion (which are rarely used for capture groups).
+ */
+predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
+  exists(RegExpParent parent | parent = term.getParent() |
+    parent instanceof RegExpAlt
+    or
+    parent instanceof RegExpQuantifier
+    or
+    parent instanceof RegExpSubPattern
+    or
+    isInsideChoiceOrSubPattern(parent)
+  )
+}
+
+/**
+ * Holds if `group` is likely to be used as a capture group.
+ */
+predicate isLikelyCaptureGroup(RegExpGroup group) {
+  group.isCapture() and
+  not isInsideChoiceOrSubPattern(group)
+}
+
+/**
+ * Holds if `seq` contains two consecutive dots `..` or escaped dots.
+ *
+ * At least one of these dots is not intended to be a subdomain separator,
+ * so we avoid flagging the pattern in this case.
+ */
+predicate hasConsecutiveDots(RegExpSequence seq) {
+  exists(int i |
+    isDotLike(seq.getChild(i)) and
+    isDotLike(seq.getChild(i + 1))
+  )
+}
+
+predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
+  seq = regexp.getAChild*() and
+  exists(RegExpDot unescapedDot, int i, string hostname |
+    hasTopLevelDomainEnding(seq, i) and
+    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
+    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
+    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
+    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
+    not hasConsecutiveDots(unescapedDot.getParent()) and
+    hostname =
+      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
+        seq.getChild(i).getRawValue()
+  |
+    if unescapedDot.getParent() instanceof RegExpQuantifier
+    then
+      // `.*\.example.com` can match `evil.com/?x=.example.com`
+      //
+      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
+      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
+      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
+      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
+      seq.getChild(0) instanceof RegExpCaret and
+      not seq.getAChild() instanceof RegExpDollar and
+      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
+      msg =
+        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
+          + "' which may cause '" + hostname +
+          "' to be matched anywhere in the URL, outside the hostname."
+    else
+      msg =
+        "has an unescaped '.' before '" + hostname +
+          "', so it might match more hosts than expected."
+  )
+}
+
+predicate incompleteHostnameRegExp(
+  RegExpSequence hostSequence, string message, DataFlow::Node aux, string label
+) {
+  exists(RegExpPatternSource re, RegExpTerm regexp, string msg, string kind |
+    regexp = re.getRegExpTerm() and
+    isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
+    (
+      if re.getAParse() != re
+      then (
+        kind = "string, which is used as a regular expression $@," and
+        aux = re.getAParse()
+      ) else (
+        kind = "regular expression" and aux = re
+      )
+    )
+  |
+    message = "This " + kind + " " + msg and label = "here"
+  )
+}
diff --git a/javascript/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll b/javascript/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
new file mode 100644
index 00000000000..7046b14f09d
--- /dev/null
+++ b/javascript/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
@@ -0,0 +1 @@
+import javascript
diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
index bd095887011..3ae844bf57f 100644
--- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
+++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
@@ -11,97 +11,6 @@
  *       external/cwe/cwe-020
  */
 
-import javascript
-import semmle.javascript.CharacterEscapes
 import HostnameRegexpShared
 
-/**
- * Holds if `term` occurs inside a quantifier or alternative (and thus
- * can not be expected to correspond to a unique match), or as part of
- * a lookaround assertion (which are rarely used for capture groups).
- */
-predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
-  exists(RegExpParent parent | parent = term.getParent() |
-    parent instanceof RegExpAlt
-    or
-    parent instanceof RegExpQuantifier
-    or
-    parent instanceof RegExpSubPattern
-    or
-    isInsideChoiceOrSubPattern(parent)
-  )
-}
-
-/**
- * Holds if `group` is likely to be used as a capture group.
- */
-predicate isLikelyCaptureGroup(RegExpGroup group) {
-  group.isCapture() and
-  not isInsideChoiceOrSubPattern(group)
-}
-
-/**
- * Holds if `seq` contains two consecutive dots `..` or escaped dots.
- *
- * At least one of these dots is not intended to be a subdomain separator,
- * so we avoid flagging the pattern in this case.
- */
-predicate hasConsecutiveDots(RegExpSequence seq) {
-  exists(int i |
-    isDotLike(seq.getChild(i)) and
-    isDotLike(seq.getChild(i + 1))
-  )
-}
-
-predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
-  seq = regexp.getAChild*() and
-  exists(RegExpDot unescapedDot, int i, string hostname |
-    hasTopLevelDomainEnding(seq, i) and
-    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
-    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
-    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
-    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
-    not hasConsecutiveDots(unescapedDot.getParent()) and
-    hostname =
-      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
-        seq.getChild(i).getRawValue()
-  |
-    if unescapedDot.getParent() instanceof RegExpQuantifier
-    then
-      // `.*\.example.com` can match `evil.com/?x=.example.com`
-      //
-      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
-      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
-      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
-      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
-      seq.getChild(0) instanceof RegExpCaret and
-      not seq.getAChild() instanceof RegExpDollar and
-      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
-      msg =
-        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
-          + "' which may cause '" + hostname +
-          "' to be matched anywhere in the URL, outside the hostname."
-    else
-      msg =
-        "has an unescaped '.' before '" + hostname +
-          "', so it might match more hosts than expected."
-  )
-}
-
-from
-  RegExpPatternSource re, RegExpTerm regexp, RegExpSequence hostSequence, string msg, string kind,
-  DataFlow::Node aux
-where
-  regexp = re.getRegExpTerm() and
-  isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
-  (
-    if re.getAParse() != re
-    then (
-      kind = "string, which is used as a regular expression $@," and
-      aux = re.getAParse()
-    ) else (
-      kind = "regular expression" and aux = re
-    )
-  ) and
-  not CharacterEscapes::hasALikelyRegExpPatternMistake(re)
-select hostSequence, "This " + kind + " " + msg, aux, "here"
+query predicate problems = incompleteHostnameRegExp/4;
diff --git a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
index 9043fa949ba..7b5017239a6 100644
--- a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
+++ b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
@@ -398,6 +398,8 @@ class RegExpAlt extends RegExpTerm, TRegExpAlt {
   override string getAPrimaryQlClass() { result = "RegExpAlt" }
 }
 
+class RegExpCharEscape = RegExpEscape;
+
 class RegExpEscape extends RegExpNormalChar {
   RegExpEscape() { re.escapedCharacter(start, end) }
 
diff --git a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
index 102d719979e..d003d4b9a10 100644
--- a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
+++ b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
@@ -3,8 +3,7 @@
  * that match URLs and hostname patterns.
  */
 
-private import codeql.ruby.security.performance.RegExpTreeView
-private import codeql.ruby.DataFlow
+private import HostnameRegexpSpecific
 
 /**
  * Holds if the given constant is unlikely to occur in the origin part of a URL.
@@ -20,7 +19,7 @@ predicate isConstantInvalidInsideOrigin(RegExpConstant term) {
 
 /** Holds if `term` is a dot constant of form `\.` or `[.]`. */
 predicate isDotConstant(RegExpTerm term) {
-  term.(RegExpEscape).getValue() = "."
+  term.(RegExpCharEscape).getValue() = "."
   or
   exists(RegExpCharacterClass cls |
     term = cls and
@@ -108,3 +107,96 @@ predicate alwaysMatchesHostnameAlt(RegExpAlt alt, int i) {
   alwaysMatchesHostnameAlt(alt, i - 1) and
   alwaysMatchesHostname(alt.getChild(i))
 }
+
+/**
+ * Holds if `term` occurs inside a quantifier or alternative (and thus
+ * can not be expected to correspond to a unique match), or as part of
+ * a lookaround assertion (which are rarely used for capture groups).
+ */
+predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
+  exists(RegExpParent parent | parent = term.getParent() |
+    parent instanceof RegExpAlt
+    or
+    parent instanceof RegExpQuantifier
+    or
+    parent instanceof RegExpSubPattern
+    or
+    isInsideChoiceOrSubPattern(parent)
+  )
+}
+
+/**
+ * Holds if `group` is likely to be used as a capture group.
+ */
+predicate isLikelyCaptureGroup(RegExpGroup group) {
+  group.isCapture() and
+  not isInsideChoiceOrSubPattern(group)
+}
+
+/**
+ * Holds if `seq` contains two consecutive dots `..` or escaped dots.
+ *
+ * At least one of these dots is not intended to be a subdomain separator,
+ * so we avoid flagging the pattern in this case.
+ */
+predicate hasConsecutiveDots(RegExpSequence seq) {
+  exists(int i |
+    isDotLike(seq.getChild(i)) and
+    isDotLike(seq.getChild(i + 1))
+  )
+}
+
+predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
+  seq = regexp.getAChild*() and
+  exists(RegExpDot unescapedDot, int i, string hostname |
+    hasTopLevelDomainEnding(seq, i) and
+    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
+    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
+    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
+    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
+    not hasConsecutiveDots(unescapedDot.getParent()) and
+    hostname =
+      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
+        seq.getChild(i).getRawValue()
+  |
+    if unescapedDot.getParent() instanceof RegExpQuantifier
+    then
+      // `.*\.example.com` can match `evil.com/?x=.example.com`
+      //
+      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
+      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
+      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
+      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
+      seq.getChild(0) instanceof RegExpCaret and
+      not seq.getAChild() instanceof RegExpDollar and
+      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
+      msg =
+        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
+          + "' which may cause '" + hostname +
+          "' to be matched anywhere in the URL, outside the hostname."
+    else
+      msg =
+        "has an unescaped '.' before '" + hostname +
+          "', so it might match more hosts than expected."
+  )
+}
+
+predicate incompleteHostnameRegExp(
+  RegExpSequence hostSequence, string message, DataFlow::Node aux, string label
+) {
+  exists(RegExpPatternSource re, RegExpTerm regexp, string msg, string kind |
+    regexp = re.getRegExpTerm() and
+    isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
+    (
+      if re.getAParse() != re
+      then (
+        kind = "string, which is used as a regular expression $@," and
+        aux = re.getAParse()
+      ) else (
+        kind = "regular expression" and aux = re
+      )
+    )
+  |
+    message = "This " + kind + " " + msg and label = "here"
+  )
+}
diff --git a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpSpecific.qll b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpSpecific.qll
new file mode 100644
index 00000000000..d01dae07b8b
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpSpecific.qll
@@ -0,0 +1,2 @@
+import codeql.ruby.security.performance.RegExpTreeView
+import codeql.ruby.DataFlow
diff --git a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
index 5526978b07a..f300fc9403f 100644
--- a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
+++ b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.ql
@@ -12,96 +12,5 @@
  */
 
 import HostnameRegexpShared
-import codeql.ruby.security.performance.RegExpTreeView
-import codeql.ruby.DataFlow
-import codeql.ruby.AST as AST
 
-/**
- * Holds if `term` occurs inside a quantifier or alternative (and thus
- * can not be expected to correspond to a unique match), or as part of
- * a lookaround assertion (which are rarely used for capture groups).
- */
-predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
-  exists(RegExpParent parent | parent = term.getParent() |
-    parent instanceof RegExpAlt
-    or
-    parent instanceof RegExpQuantifier
-    or
-    parent instanceof RegExpSubPattern
-    or
-    isInsideChoiceOrSubPattern(parent)
-  )
-}
-
-/**
- * Holds if `group` is likely to be used as a capture group.
- */
-predicate isLikelyCaptureGroup(RegExpGroup group) {
-  group.isCapture() and
-  not isInsideChoiceOrSubPattern(group)
-}
-
-/**
- * Holds if `seq` contains two consecutive dots `..` or escaped dots.
- *
- * At least one of these dots is not intended to be a subdomain separator,
- * so we avoid flagging the pattern in this case.
- */
-predicate hasConsecutiveDots(RegExpSequence seq) {
-  exists(int i |
-    isDotLike(seq.getChild(i)) and
-    isDotLike(seq.getChild(i + 1))
-  )
-}
-
-predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
-  seq = regexp.getAChild*() and
-  exists(RegExpDot unescapedDot, int i, string hostname |
-    hasTopLevelDomainEnding(seq, i) and
-    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
-    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
-    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
-    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
-    not hasConsecutiveDots(unescapedDot.getParent()) and
-    hostname =
-      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
-        seq.getChild(i).getRawValue()
-  |
-    if unescapedDot.getParent() instanceof RegExpQuantifier
-    then
-      // `.*\.example.com` can match `evil.com/?x=.example.com`
-      //
-      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
-      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
-      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
-      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
-      seq.getChild(0) instanceof RegExpCaret and
-      not seq.getAChild() instanceof RegExpDollar and
-      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
-      msg =
-        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
-          + "' which may cause '" + hostname +
-          "' to be matched anywhere in the URL, outside the hostname."
-    else
-      msg =
-        "has an unescaped '.' before '" + hostname +
-          "', so it might match more hosts than expected."
-  )
-}
-
-from
-  RegExpPatternSource re, RegExpTerm regexp, RegExpSequence hostSequence, string msg, string kind,
-  DataFlow::Node aux
-where
-  regexp = re.getRegExpTerm() and
-  isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
-  (
-    if re.getAParse() != re
-    then (
-      kind = "string, which is used as a regular expression $@," and
-      aux = re.getAParse()
-    ) else (
-      kind = "regular expression" and aux = re
-    )
-  )
-select hostSequence, "This " + kind + " " + msg, aux, "here"
+query predicate problems = incompleteHostnameRegExp/4;

From bb348116ab515c9a218f27603829ba79420d7137 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 4 Mar 2022 13:48:24 +0100
Subject: [PATCH 06/13] JavaScript: update expected output

---
 .../Security/CWE-020/IncompleteHostnameRegExp.expected          | 1 +
 .../Security/CWE-020/tst-IncompleteHostnameRegExp.js            | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

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 13950be73ba..d0fb91322fe 100644
--- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected
@@ -23,4 +23,5 @@
 | 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 |
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 df887092d23..ddc267ebdd7 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
@@ -52,7 +52,7 @@
 
 	new RegExp('test.' + 'example.com$'); // NOT OK
 
-	new RegExp('^http://test\.example.com'); // NOT OK, but flagged by js/useless-regexp-character-escape
+	new RegExp('^http://test\.example.com'); // NOT OK
 
 	/^http:\/\/(..|...)\.example\.com\/index\.html/; // OK, wildcards are intentional
 	/^http:\/\/.\.example\.com\/index\.html/; // OK, the wildcard is intentional

From ce50f35ddaa0598272624032cd72aed8ae539b7f Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Thu, 3 Mar 2022 17:34:53 +0100
Subject: [PATCH 07/13] Python: switch to shared implementation of
 IncompleteHostnameRegExp.ql

---
 config/identical-files.json                   |   3 +-
 python/ql/lib/semmle/python/RegexTreeView.qll |  40 ++++
 .../Security/CWE-020/HostnameRegexpShared.qll | 202 ++++++++++++++++++
 .../CWE-020/HostnameRegexpSpecific.qll        |   2 +
 .../CWE-020/IncompleteHostnameRegExp.ql       |  32 +--
 .../IncompleteHostnameRegExp.expected         |   2 +-
 6 files changed, 250 insertions(+), 31 deletions(-)
 create mode 100644 python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
 create mode 100644 python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll

diff --git a/config/identical-files.json b/config/identical-files.json
index a7db98e94eb..daf764f15c6 100644
--- a/config/identical-files.json
+++ b/config/identical-files.json
@@ -511,6 +511,7 @@
   ],
   "Hostname Regexp queries": [
     "javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll",
-    "ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll"
+    "ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll",
+    "python/ql/src/Security/CWE-020/HostnameRegexpShared.qll"
   ]
 }
diff --git a/python/ql/lib/semmle/python/RegexTreeView.qll b/python/ql/lib/semmle/python/RegexTreeView.qll
index 95d983f5e88..37d6d307ef4 100644
--- a/python/ql/lib/semmle/python/RegexTreeView.qll
+++ b/python/ql/lib/semmle/python/RegexTreeView.qll
@@ -2,6 +2,7 @@
 
 import python
 private import semmle.python.regex
+private import semmle.python.dataflow.new.DataFlow
 
 /**
  * An element containing a regular expression term, that is, either
@@ -48,6 +49,19 @@ newtype TRegExpParent =
   /** A back reference */
   TRegExpBackRef(Regex re, int start, int end) { re.backreference(start, end) }
 
+/**
+ * Provides regular expression patterns.
+ */
+module RegExpPatterns {
+  /**
+   * Gets a pattern that matches common top-level domain names in lower case.
+   */
+  string commonTLD() {
+    // according to ranking by http://google.com/search?q=site:.<<TLD>>
+    result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
+  }
+}
+
 /**
  * An element containing a regular expression term, that is, either
  * a string literal (parsed as a regular expression)
@@ -445,6 +459,8 @@ class RegExpAlt extends RegExpTerm, TRegExpAlt {
   override string getPrimaryQLClass() { result = "RegExpAlt" }
 }
 
+class RegExpCharEscape = RegExpEscape;
+
 /**
  * An escaped regular expression term, that is, a regular expression
  * term starting with a backslash, which is not a backreference.
@@ -751,6 +767,9 @@ class RegExpGroup extends RegExpTerm, TRegExpGroup {
    */
   int getNumber() { result = re.getGroupNumber(start, end) }
 
+  /** Holds if this is a capture group. */
+  predicate isCapture() { not exists(this.getNumber()) }
+
   /** Holds if this is a named capture group. */
   predicate isNamed() { exists(this.getName()) }
 
@@ -1009,3 +1028,24 @@ class RegExpBackRef extends RegExpTerm, TRegExpBackRef {
 
 /** Gets the parse tree resulting from parsing `re`, if such has been constructed. */
 RegExpTerm getParsedRegExp(StrConst re) { result.getRegex() = re and result.isRootTerm() }
+
+/**
+ * A node whose value may flow to a position where it is interpreted
+ * as a part of a regular expression.
+ */
+class RegExpPatternSource extends DataFlow::CfgNode {
+  private Regex astNode;
+
+  RegExpPatternSource() { astNode = this.asExpr() }
+
+  /**
+   * Gets a node where the pattern of this node is parsed as a part of
+   * a regular expression.
+   */
+  DataFlow::Node getAParse() { result = this }
+
+  /**
+   * Gets the root term of the regular expression parsed from this pattern.
+   */
+  RegExpTerm getRegExpTerm() { result.getRegex() = astNode }
+}
diff --git a/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll b/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
new file mode 100644
index 00000000000..d003d4b9a10
--- /dev/null
+++ b/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
@@ -0,0 +1,202 @@
+/**
+ * Provides predicates for reasoning about regular expressions
+ * that match URLs and hostname patterns.
+ */
+
+private import HostnameRegexpSpecific
+
+/**
+ * Holds if the given constant is unlikely to occur in the origin part of a URL.
+ */
+predicate isConstantInvalidInsideOrigin(RegExpConstant term) {
+  // Look for any of these cases:
+  // - A character that can't occur in the origin
+  // - Two dashes in a row
+  // - A colon that is not part of port or scheme separator
+  // - A slash that is not part of scheme separator
+  term.getValue().regexpMatch(".*(?:[^a-zA-Z0-9.:/-]|--|:[^0-9/]|(?<![/:]|^)/).*")
+}
+
+/** Holds if `term` is a dot constant of form `\.` or `[.]`. */
+predicate isDotConstant(RegExpTerm term) {
+  term.(RegExpCharEscape).getValue() = "."
+  or
+  exists(RegExpCharacterClass cls |
+    term = cls and
+    not cls.isInverted() and
+    cls.getNumChild() = 1 and
+    cls.getAChild().(RegExpConstant).getValue() = "."
+  )
+}
+
+/** Holds if `term` is a wildcard `.` or an actual `.` character. */
+predicate isDotLike(RegExpTerm term) {
+  term instanceof RegExpDot
+  or
+  isDotConstant(term)
+}
+
+/** Holds if `term` will only ever be matched against the beginning of the input. */
+predicate matchesBeginningOfString(RegExpTerm term) {
+  term.isRootTerm()
+  or
+  exists(RegExpTerm parent | matchesBeginningOfString(parent) |
+    term = parent.(RegExpSequence).getChild(0)
+    or
+    parent.(RegExpSequence).getChild(0) instanceof RegExpCaret and
+    term = parent.(RegExpSequence).getChild(1)
+    or
+    term = parent.(RegExpAlt).getAChild()
+    or
+    term = parent.(RegExpGroup).getAChild()
+  )
+}
+
+/**
+ * Holds if the given sequence contains top-level domain preceded by a dot, such as `.com`,
+ * excluding cases where this is at the very beginning of the regexp.
+ *
+ * `i` is bound to the index of the last child in the top-level domain part.
+ */
+predicate hasTopLevelDomainEnding(RegExpSequence seq, int i) {
+  seq.getChild(i)
+      .(RegExpConstant)
+      .getValue()
+      .regexpMatch("(?i)" + RegExpPatterns::commonTLD() + "(:\\d+)?([/?#].*)?") and
+  isDotLike(seq.getChild(i - 1)) and
+  not (i = 1 and matchesBeginningOfString(seq))
+}
+
+/**
+ * Holds if the given regular expression term contains top-level domain preceded by a dot,
+ * such as `.com`.
+ */
+predicate hasTopLevelDomainEnding(RegExpSequence seq) { hasTopLevelDomainEnding(seq, _) }
+
+/**
+ * Holds if `term` will always match a hostname, that is, all disjunctions contain
+ * a hostname pattern that isn't inside a quantifier.
+ */
+predicate alwaysMatchesHostname(RegExpTerm term) {
+  hasTopLevelDomainEnding(term, _)
+  or
+  // `localhost` is considered a hostname pattern, but has no TLD
+  term.(RegExpConstant).getValue().regexpMatch("\\blocalhost\\b")
+  or
+  not term instanceof RegExpAlt and
+  not term instanceof RegExpQuantifier and
+  alwaysMatchesHostname(term.getAChild())
+  or
+  alwaysMatchesHostnameAlt(term)
+}
+
+/** Holds if every child of `alt` contains a hostname pattern. */
+predicate alwaysMatchesHostnameAlt(RegExpAlt alt) {
+  alwaysMatchesHostnameAlt(alt, alt.getNumChild() - 1)
+}
+
+/**
+ * Holds if the first `i` children of `alt` contains a hostname pattern.
+ *
+ * This is used instead of `forall` to avoid materializing the set of alternatives
+ * that don't contains hostnames, which is much larger.
+ */
+predicate alwaysMatchesHostnameAlt(RegExpAlt alt, int i) {
+  alwaysMatchesHostname(alt.getChild(0)) and i = 0
+  or
+  alwaysMatchesHostnameAlt(alt, i - 1) and
+  alwaysMatchesHostname(alt.getChild(i))
+}
+
+/**
+ * Holds if `term` occurs inside a quantifier or alternative (and thus
+ * can not be expected to correspond to a unique match), or as part of
+ * a lookaround assertion (which are rarely used for capture groups).
+ */
+predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
+  exists(RegExpParent parent | parent = term.getParent() |
+    parent instanceof RegExpAlt
+    or
+    parent instanceof RegExpQuantifier
+    or
+    parent instanceof RegExpSubPattern
+    or
+    isInsideChoiceOrSubPattern(parent)
+  )
+}
+
+/**
+ * Holds if `group` is likely to be used as a capture group.
+ */
+predicate isLikelyCaptureGroup(RegExpGroup group) {
+  group.isCapture() and
+  not isInsideChoiceOrSubPattern(group)
+}
+
+/**
+ * Holds if `seq` contains two consecutive dots `..` or escaped dots.
+ *
+ * At least one of these dots is not intended to be a subdomain separator,
+ * so we avoid flagging the pattern in this case.
+ */
+predicate hasConsecutiveDots(RegExpSequence seq) {
+  exists(int i |
+    isDotLike(seq.getChild(i)) and
+    isDotLike(seq.getChild(i + 1))
+  )
+}
+
+predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
+  seq = regexp.getAChild*() and
+  exists(RegExpDot unescapedDot, int i, string hostname |
+    hasTopLevelDomainEnding(seq, i) and
+    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
+    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
+    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
+    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
+    not hasConsecutiveDots(unescapedDot.getParent()) and
+    hostname =
+      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
+        seq.getChild(i).getRawValue()
+  |
+    if unescapedDot.getParent() instanceof RegExpQuantifier
+    then
+      // `.*\.example.com` can match `evil.com/?x=.example.com`
+      //
+      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
+      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
+      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
+      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
+      seq.getChild(0) instanceof RegExpCaret and
+      not seq.getAChild() instanceof RegExpDollar and
+      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
+      msg =
+        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
+          + "' which may cause '" + hostname +
+          "' to be matched anywhere in the URL, outside the hostname."
+    else
+      msg =
+        "has an unescaped '.' before '" + hostname +
+          "', so it might match more hosts than expected."
+  )
+}
+
+predicate incompleteHostnameRegExp(
+  RegExpSequence hostSequence, string message, DataFlow::Node aux, string label
+) {
+  exists(RegExpPatternSource re, RegExpTerm regexp, string msg, string kind |
+    regexp = re.getRegExpTerm() and
+    isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
+    (
+      if re.getAParse() != re
+      then (
+        kind = "string, which is used as a regular expression $@," and
+        aux = re.getAParse()
+      ) else (
+        kind = "regular expression" and aux = re
+      )
+    )
+  |
+    message = "This " + kind + " " + msg and label = "here"
+  )
+}
diff --git a/python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll b/python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
new file mode 100644
index 00000000000..2e13beb440b
--- /dev/null
+++ b/python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
@@ -0,0 +1,2 @@
+import semmle.python.security.performance.RegExpTreeView
+import semmle.python.dataflow.new.DataFlow
diff --git a/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
index 7f32419261c..db7129e5ac5 100644
--- a/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
+++ b/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
@@ -8,35 +8,9 @@
  * @id py/incomplete-hostname-regexp
  * @tags correctness
  *       security
- *       external/cwe/cwe-20
+ *       external/cwe/cwe-020
  */
 
-import python
-import semmle.python.regex
+import HostnameRegexpShared
 
-private string commonTopLevelDomainRegex() { result = "com|org|edu|gov|uk|net|io" }
-
-/**
- * Holds if `pattern` is a regular expression pattern for URLs with a host matched by `hostPart`,
- * and `pattern` contains a subtle mistake that allows it to match unexpected hosts.
- */
-bindingset[pattern]
-predicate isIncompleteHostNameRegExpPattern(string pattern, string hostPart) {
-  hostPart =
-    pattern
-        .regexpCapture("(?i).*" +
-            // an unescaped single `.`
-            "(?<!\\\\)[.]" +
-            // immediately followed by a sequence of subdomains, perhaps with some regex characters mixed in, followed by a known TLD
-            "([():|?a-z0-9-]+(\\\\)?[.](" + commonTopLevelDomainRegex() + "))" + ".*", 1)
-}
-
-from Regex r, string pattern, string hostPart
-where
-  r.getText() = pattern and
-  isIncompleteHostNameRegExpPattern(pattern, hostPart) and
-  // ignore patterns with capture groups after the TLD
-  not pattern.regexpMatch("(?i).*[.](" + commonTopLevelDomainRegex() + ").*[(][?]:.*[)].*")
-select r,
-  "This regular expression has an unescaped '.' before '" + hostPart +
-    "', so it might match more hosts than expected."
+query predicate problems = incompleteHostnameRegExp/4;
diff --git a/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
index 00e0fcb95a0..6a3c6769c35 100644
--- a/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
+++ b/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
@@ -1 +1 @@
-| hosttest.py:6:27:6:51 | Str | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. |
+| hosttest.py:6:31:6:53 | (www\|beta).example.com/ | This regular expression has an unescaped '.' before 'example.com/', so it might match more hosts than expected. | hosttest.py:6:27:6:51 | ControlFlowNode for Str | here |

From cf4b83453656ed85da32ddc77cd57a957165a95d Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 11 Mar 2022 14:25:34 +0100
Subject: [PATCH 08/13] Address comments

---
 javascript/ql/lib/semmle/javascript/Regexp.qll              | 4 ++--
 javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll | 2 +-
 .../Security/CWE-020/IncompleteUrlSubstringSanitization.ql  | 2 +-
 python/ql/lib/semmle/python/RegexTreeView.qll               | 6 +++---
 python/ql/src/Security/CWE-020/HostnameRegexpShared.qll     | 2 +-
 .../lib/codeql/ruby/security/performance/RegExpTreeView.qll | 6 +++---
 .../src/queries/security/cwe-020/HostnameRegexpShared.qll   | 2 +-
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/Regexp.qll b/javascript/ql/lib/semmle/javascript/Regexp.qll
index ddb50436fb8..33bc76d9dca 100644
--- a/javascript/ql/lib/semmle/javascript/Regexp.qll
+++ b/javascript/ql/lib/semmle/javascript/Regexp.qll
@@ -999,13 +999,13 @@ predicate isInterpretedAsRegExp(DataFlow::Node source) {
 }
 
 /**
- * Provides regular expression patterns.
+ * Provides utility predicates related to regular expressions.
  */
 module RegExpPatterns {
   /**
    * Gets a pattern that matches common top-level domain names in lower case.
    */
-  string commonTLD() {
+  string getACommonTld() {
     // according to ranking by http://google.com/search?q=site:.<<TLD>>
     result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
   }
diff --git a/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll b/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll
index d003d4b9a10..2192951f76d 100644
--- a/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll
+++ b/javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll
@@ -62,7 +62,7 @@ predicate hasTopLevelDomainEnding(RegExpSequence seq, int i) {
   seq.getChild(i)
       .(RegExpConstant)
       .getValue()
-      .regexpMatch("(?i)" + RegExpPatterns::commonTLD() + "(:\\d+)?([/?#].*)?") and
+      .regexpMatch("(?i)" + RegExpPatterns::getACommonTld() + "(:\\d+)?([/?#].*)?") and
   isDotLike(seq.getChild(i - 1)) and
   not (i = 1 and matchesBeginningOfString(seq))
 }
diff --git a/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql b/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql
index 56ab631d1e1..466022784f4 100644
--- a/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql
+++ b/javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql
@@ -39,7 +39,7 @@ where
   (
     // target contains a domain on a common TLD, and perhaps some other URL components
     target
-        .regexpMatch("(?i)([a-z]*:?//)?\\.?([a-z0-9-]+\\.)+" + RegExpPatterns::commonTLD() +
+        .regexpMatch("(?i)([a-z]*:?//)?\\.?([a-z0-9-]+\\.)+" + RegExpPatterns::getACommonTld() +
             "(:[0-9]+)?/?")
     or
     // target is a HTTP URL to a domain on any TLD
diff --git a/python/ql/lib/semmle/python/RegexTreeView.qll b/python/ql/lib/semmle/python/RegexTreeView.qll
index 37d6d307ef4..019089ecbc2 100644
--- a/python/ql/lib/semmle/python/RegexTreeView.qll
+++ b/python/ql/lib/semmle/python/RegexTreeView.qll
@@ -50,13 +50,13 @@ newtype TRegExpParent =
   TRegExpBackRef(Regex re, int start, int end) { re.backreference(start, end) }
 
 /**
- * Provides regular expression patterns.
+ * Provides utility predicates related to regular expressions.
  */
 module RegExpPatterns {
   /**
    * Gets a pattern that matches common top-level domain names in lower case.
    */
-  string commonTLD() {
+  string getACommonTld() {
     // according to ranking by http://google.com/search?q=site:.<<TLD>>
     result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
   }
@@ -768,7 +768,7 @@ class RegExpGroup extends RegExpTerm, TRegExpGroup {
   int getNumber() { result = re.getGroupNumber(start, end) }
 
   /** Holds if this is a capture group. */
-  predicate isCapture() { not exists(this.getNumber()) }
+  predicate isCapture() { exists(this.getNumber()) }
 
   /** Holds if this is a named capture group. */
   predicate isNamed() { exists(this.getName()) }
diff --git a/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll b/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
index d003d4b9a10..2192951f76d 100644
--- a/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
+++ b/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
@@ -62,7 +62,7 @@ predicate hasTopLevelDomainEnding(RegExpSequence seq, int i) {
   seq.getChild(i)
       .(RegExpConstant)
       .getValue()
-      .regexpMatch("(?i)" + RegExpPatterns::commonTLD() + "(:\\d+)?([/?#].*)?") and
+      .regexpMatch("(?i)" + RegExpPatterns::getACommonTld() + "(:\\d+)?([/?#].*)?") and
   isDotLike(seq.getChild(i - 1)) and
   not (i = 1 and matchesBeginningOfString(seq))
 }
diff --git a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
index 7b5017239a6..c059f7f131c 100644
--- a/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
+++ b/ruby/ql/lib/codeql/ruby/security/performance/RegExpTreeView.qll
@@ -60,13 +60,13 @@ module RegExpFlags {
 }
 
 /**
- * Provides regular expression patterns.
+ * Provides utility predicates related to regular expressions.
  */
 module RegExpPatterns {
   /**
    * Gets a pattern that matches common top-level domain names in lower case.
    */
-  string commonTLD() {
+  string getACommonTld() {
     // according to ranking by http://google.com/search?q=site:.<<TLD>>
     result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
   }
@@ -640,7 +640,7 @@ class RegExpGroup extends RegExpTerm, TRegExpGroup {
   int getNumber() { result = re.getGroupNumber(start, end) }
 
   /** Holds if this is a capture group. */
-  predicate isCapture() { not exists(this.getNumber()) }
+  predicate isCapture() { exists(this.getNumber()) }
 
   /** Holds if this is a named capture group. */
   predicate isNamed() { exists(this.getName()) }
diff --git a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
index d003d4b9a10..2192951f76d 100644
--- a/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
+++ b/ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll
@@ -62,7 +62,7 @@ predicate hasTopLevelDomainEnding(RegExpSequence seq, int i) {
   seq.getChild(i)
       .(RegExpConstant)
       .getValue()
-      .regexpMatch("(?i)" + RegExpPatterns::commonTLD() + "(:\\d+)?([/?#].*)?") and
+      .regexpMatch("(?i)" + RegExpPatterns::getACommonTld() + "(:\\d+)?([/?#].*)?") and
   isDotLike(seq.getChild(i - 1)) and
   not (i = 1 and matchesBeginningOfString(seq))
 }

From 852f05bfb7aa32edf70a753d0d2b90d5e228b34c Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 16 Mar 2022 12:26:39 +0100
Subject: [PATCH 09/13] Address comment

---
 javascript/ql/lib/semmle/javascript/Regexp.qll | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/javascript/ql/lib/semmle/javascript/Regexp.qll b/javascript/ql/lib/semmle/javascript/Regexp.qll
index 33bc76d9dca..a208a98753f 100644
--- a/javascript/ql/lib/semmle/javascript/Regexp.qll
+++ b/javascript/ql/lib/semmle/javascript/Regexp.qll
@@ -1009,6 +1009,12 @@ module RegExpPatterns {
     // according to ranking by http://google.com/search?q=site:.<<TLD>>
     result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
   }
+
+  /**
+   * Gets a pattern that matches common top-level domain names in lower case.
+   * DEPRECATED: use `getACommonTld` instead
+   */
+  deprecated predicate commonTLD = getACommonTld/0;
 }
 
 /**

From 6b323eeda86970a08fbd53ab2167aa82868beed5 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 16 Mar 2022 12:34:03 +0100
Subject: [PATCH 10/13] Update expected output

---
 .../IncompleteHostnameRegExp.expected                         | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
index f70cbc19ce7..8ab11cff1d6 100644
--- a/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
+++ b/ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
@@ -10,6 +10,7 @@
 | tst-IncompleteHostnameRegExp.rb: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.rb:10:2:10:37 | /^http:\\/\\/test.example.com\\/(.../ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:11:13:11:38 | "^http://test.example.com" | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:12:14:12:39 | "^http://test.example.com" | here |
+| tst-IncompleteHostnameRegExp.rb: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.rb:15:13:15:50 | call to id | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:17:13:17:31 | `test.example.com$` | here |
 | tst-IncompleteHostnameRegExp.rb:19:14:19:30 | ^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.rb:20:13:20:26 | "#{...}$" | here |
 | tst-IncompleteHostnameRegExp.rb:20:14:20:31 | ^test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:20:13:20:26 | "#{...}$" | here |
@@ -17,8 +18,11 @@
 | tst-IncompleteHostnameRegExp.rb: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.rb:38:2:38:44 | /^(http\|https):\\/\\/www.example.../ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:39:2:39:33 | /^(http:\\/\\/sub.example.com\\/)/ | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:40:2:40:30 | /^https?:\\/\\/api.example.com/ | here |
+| tst-IncompleteHostnameRegExp.rb:41:42:41:68 | ^https?://.+\\.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:41:13:41:71 | ... + ... | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:43:2:43:33 | /^https:\\/\\/[a-z]*.example.com$/ | here |
 | tst-IncompleteHostnameRegExp.rb:44:40:44:53 | .+.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
 | tst-IncompleteHostnameRegExp.rb:44:55:44:70 | .+.example-a.com | This regular expression has an unescaped '.' before 'example-a.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
 | tst-IncompleteHostnameRegExp.rb:44:72:44:87 | .+.example-b.com | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:44:17:44:109 | "^protos?://(localhost\|.+.exam..." | here |
+| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.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.rb:48:13:48:69 | ... + ... | here |
+| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:48:13:48:69 | ... + ... | here |
 | tst-IncompleteHostnameRegExp.rb: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.rb:59:2:59:32 | /^(foo.example\\.com\|whatever)$/ | here |

From 1a51f0cf568d0da51604ad338b27677a09de617b Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 16 Mar 2022 14:32:05 +0100
Subject: [PATCH 11/13] Ruby: regex: fix getGroupNumber

non-capture groups should not have a group number
---
 ruby/ql/lib/codeql/ruby/security/performance/ParseRegExp.qll   | 3 ++-
 ruby/ql/test/library-tests/regexp/regexp.expected              | 3 ---
 .../ql/test/query-tests/security/cwe-116/BadTagFilter.expected | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/security/performance/ParseRegExp.qll b/ruby/ql/lib/codeql/ruby/security/performance/ParseRegExp.qll
index b40c1fc4d50..52265e1c445 100644
--- a/ruby/ql/lib/codeql/ruby/security/performance/ParseRegExp.qll
+++ b/ruby/ql/lib/codeql/ruby/security/performance/ParseRegExp.qll
@@ -480,6 +480,7 @@ abstract class RegExp extends AST::StringlikeLiteral {
   /** Gets the number of the group in start,end */
   int getGroupNumber(int start, int end) {
     this.group(start, end) and
+    not this.nonCapturingGroupStart(start, _) and
     result =
       count(int i | this.group(i, _) and i < start and not this.nonCapturingGroupStart(i, _)) + 1
   }
@@ -583,7 +584,7 @@ abstract class RegExp extends AST::StringlikeLiteral {
   private predicate nonCapturingGroupStart(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
-    this.getChar(start + 2) = ":" and
+    this.getChar(start + 2) = [":", "=", "<", "!", "#"] and
     end = start + 3
   }
 
diff --git a/ruby/ql/test/library-tests/regexp/regexp.expected b/ruby/ql/test/library-tests/regexp/regexp.expected
index 35c85b0132f..4d1837bedea 100644
--- a/ruby/ql/test/library-tests/regexp/regexp.expected
+++ b/ruby/ql/test/library-tests/regexp/regexp.expected
@@ -6,11 +6,8 @@ groupNumber
 | regexp.rb:46:2:46:6 | (foo) | 1 |
 | regexp.rb:47:4:47:8 | (o\|b) | 1 |
 | regexp.rb:48:2:48:9 | (a\|b\|cd) | 1 |
-| regexp.rb:49:2:49:7 | (?::+) | 1 |
-| regexp.rb:52:2:52:11 | (?<id>\\w+) | 1 |
 | regexp.rb:53:2:53:12 | (?'foo'fo+) | 1 |
 | regexp.rb:56:2:56:5 | (a+) | 1 |
-| regexp.rb:57:2:57:11 | (?<qux>q+) | 1 |
 term
 | regexp.rb:5:2:5:4 | abc | RegExpConstant,RegExpNormalChar |
 | regexp.rb:8:2:8:2 | a | RegExpConstant,RegExpNormalChar |
diff --git a/ruby/ql/test/query-tests/security/cwe-116/BadTagFilter.expected b/ruby/ql/test/query-tests/security/cwe-116/BadTagFilter.expected
index fe1b106a083..edd6347e2d3 100644
--- a/ruby/ql/test/query-tests/security/cwe-116/BadTagFilter.expected
+++ b/ruby/ql/test/query-tests/security/cwe-116/BadTagFilter.expected
@@ -11,4 +11,4 @@
 | test.rb:15:6:15:39 | <script[^>]*?>[\\s\\S]*?<\\/script.*> | This regular expression does not match script end tags like </script\\t\\n bar>. |
 | test.rb:17:6:17:40 | <script\\b[^>]*>([\\s\\S]*?)<\\/script> | This regular expression does not match script end tags like </script >. |
 | test.rb:18:6:18:48 | <(?:!--([\\S\|\\s]*?)-->)\|([^\\/\\s>]+)[\\S\\s]*?> | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 1 and comments ending with --!> are matched with capture group 2. |
-| test.rb:19:6:19:147 | <(?:(?:\\/([^>]+)>)\|(?:!--([\\S\|\\s]*?)-->)\|(?:([^\\/\\s>]+)((?:\\s+[\\w\\-:.]+(?:\\s*=\\s*?(?:(?:"[^"]*")\|(?:'[^']*')\|[^\\s"'\\/>]+))?)*)[\\S\\s]*?(\\/?)>)) | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 2 and comments ending with --!> are matched with capture group 1, 3, 4, 5. |
+| test.rb:19:6:19:147 | <(?:(?:\\/([^>]+)>)\|(?:!--([\\S\|\\s]*?)-->)\|(?:([^\\/\\s>]+)((?:\\s+[\\w\\-:.]+(?:\\s*=\\s*?(?:(?:"[^"]*")\|(?:'[^']*')\|[^\\s"'\\/>]+))?)*)[\\S\\s]*?(\\/?)>)) | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 2 and comments ending with --!> are matched with capture group 3, 4. |

From 6d245914168abc5d7e1cb610295a0d6890ca8749 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 18 Mar 2022 13:02:55 +0100
Subject: [PATCH 12/13] Revert "Python: switch to shared implementation of
 IncompleteHostnameRegExp.ql"

This reverts commit ce50f35ddaa0598272624032cd72aed8ae539b7f.
---
 config/identical-files.json                   |   3 +-
 python/ql/lib/semmle/python/RegexTreeView.qll |  40 ----
 .../Security/CWE-020/HostnameRegexpShared.qll | 202 ------------------
 .../CWE-020/HostnameRegexpSpecific.qll        |   2 -
 .../CWE-020/IncompleteHostnameRegExp.ql       |  32 ++-
 .../IncompleteHostnameRegExp.expected         |   2 +-
 6 files changed, 31 insertions(+), 250 deletions(-)
 delete mode 100644 python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
 delete mode 100644 python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll

diff --git a/config/identical-files.json b/config/identical-files.json
index ff4129d9776..75b86933ebb 100644
--- a/config/identical-files.json
+++ b/config/identical-files.json
@@ -518,7 +518,6 @@
   ],
   "Hostname Regexp queries": [
     "javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll",
-    "ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll",
-    "python/ql/src/Security/CWE-020/HostnameRegexpShared.qll"
+    "ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll"
   ]
 }
\ No newline at end of file
diff --git a/python/ql/lib/semmle/python/RegexTreeView.qll b/python/ql/lib/semmle/python/RegexTreeView.qll
index 019089ecbc2..95d983f5e88 100644
--- a/python/ql/lib/semmle/python/RegexTreeView.qll
+++ b/python/ql/lib/semmle/python/RegexTreeView.qll
@@ -2,7 +2,6 @@
 
 import python
 private import semmle.python.regex
-private import semmle.python.dataflow.new.DataFlow
 
 /**
  * An element containing a regular expression term, that is, either
@@ -49,19 +48,6 @@ newtype TRegExpParent =
   /** A back reference */
   TRegExpBackRef(Regex re, int start, int end) { re.backreference(start, end) }
 
-/**
- * Provides utility predicates related to regular expressions.
- */
-module RegExpPatterns {
-  /**
-   * Gets a pattern that matches common top-level domain names in lower case.
-   */
-  string getACommonTld() {
-    // according to ranking by http://google.com/search?q=site:.<<TLD>>
-    result = "(?:com|org|edu|gov|uk|net|io)(?![a-z0-9])"
-  }
-}
-
 /**
  * An element containing a regular expression term, that is, either
  * a string literal (parsed as a regular expression)
@@ -459,8 +445,6 @@ class RegExpAlt extends RegExpTerm, TRegExpAlt {
   override string getPrimaryQLClass() { result = "RegExpAlt" }
 }
 
-class RegExpCharEscape = RegExpEscape;
-
 /**
  * An escaped regular expression term, that is, a regular expression
  * term starting with a backslash, which is not a backreference.
@@ -767,9 +751,6 @@ class RegExpGroup extends RegExpTerm, TRegExpGroup {
    */
   int getNumber() { result = re.getGroupNumber(start, end) }
 
-  /** Holds if this is a capture group. */
-  predicate isCapture() { exists(this.getNumber()) }
-
   /** Holds if this is a named capture group. */
   predicate isNamed() { exists(this.getName()) }
 
@@ -1028,24 +1009,3 @@ class RegExpBackRef extends RegExpTerm, TRegExpBackRef {
 
 /** Gets the parse tree resulting from parsing `re`, if such has been constructed. */
 RegExpTerm getParsedRegExp(StrConst re) { result.getRegex() = re and result.isRootTerm() }
-
-/**
- * A node whose value may flow to a position where it is interpreted
- * as a part of a regular expression.
- */
-class RegExpPatternSource extends DataFlow::CfgNode {
-  private Regex astNode;
-
-  RegExpPatternSource() { astNode = this.asExpr() }
-
-  /**
-   * Gets a node where the pattern of this node is parsed as a part of
-   * a regular expression.
-   */
-  DataFlow::Node getAParse() { result = this }
-
-  /**
-   * Gets the root term of the regular expression parsed from this pattern.
-   */
-  RegExpTerm getRegExpTerm() { result.getRegex() = astNode }
-}
diff --git a/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll b/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
deleted file mode 100644
index 2192951f76d..00000000000
--- a/python/ql/src/Security/CWE-020/HostnameRegexpShared.qll
+++ /dev/null
@@ -1,202 +0,0 @@
-/**
- * Provides predicates for reasoning about regular expressions
- * that match URLs and hostname patterns.
- */
-
-private import HostnameRegexpSpecific
-
-/**
- * Holds if the given constant is unlikely to occur in the origin part of a URL.
- */
-predicate isConstantInvalidInsideOrigin(RegExpConstant term) {
-  // Look for any of these cases:
-  // - A character that can't occur in the origin
-  // - Two dashes in a row
-  // - A colon that is not part of port or scheme separator
-  // - A slash that is not part of scheme separator
-  term.getValue().regexpMatch(".*(?:[^a-zA-Z0-9.:/-]|--|:[^0-9/]|(?<![/:]|^)/).*")
-}
-
-/** Holds if `term` is a dot constant of form `\.` or `[.]`. */
-predicate isDotConstant(RegExpTerm term) {
-  term.(RegExpCharEscape).getValue() = "."
-  or
-  exists(RegExpCharacterClass cls |
-    term = cls and
-    not cls.isInverted() and
-    cls.getNumChild() = 1 and
-    cls.getAChild().(RegExpConstant).getValue() = "."
-  )
-}
-
-/** Holds if `term` is a wildcard `.` or an actual `.` character. */
-predicate isDotLike(RegExpTerm term) {
-  term instanceof RegExpDot
-  or
-  isDotConstant(term)
-}
-
-/** Holds if `term` will only ever be matched against the beginning of the input. */
-predicate matchesBeginningOfString(RegExpTerm term) {
-  term.isRootTerm()
-  or
-  exists(RegExpTerm parent | matchesBeginningOfString(parent) |
-    term = parent.(RegExpSequence).getChild(0)
-    or
-    parent.(RegExpSequence).getChild(0) instanceof RegExpCaret and
-    term = parent.(RegExpSequence).getChild(1)
-    or
-    term = parent.(RegExpAlt).getAChild()
-    or
-    term = parent.(RegExpGroup).getAChild()
-  )
-}
-
-/**
- * Holds if the given sequence contains top-level domain preceded by a dot, such as `.com`,
- * excluding cases where this is at the very beginning of the regexp.
- *
- * `i` is bound to the index of the last child in the top-level domain part.
- */
-predicate hasTopLevelDomainEnding(RegExpSequence seq, int i) {
-  seq.getChild(i)
-      .(RegExpConstant)
-      .getValue()
-      .regexpMatch("(?i)" + RegExpPatterns::getACommonTld() + "(:\\d+)?([/?#].*)?") and
-  isDotLike(seq.getChild(i - 1)) and
-  not (i = 1 and matchesBeginningOfString(seq))
-}
-
-/**
- * Holds if the given regular expression term contains top-level domain preceded by a dot,
- * such as `.com`.
- */
-predicate hasTopLevelDomainEnding(RegExpSequence seq) { hasTopLevelDomainEnding(seq, _) }
-
-/**
- * Holds if `term` will always match a hostname, that is, all disjunctions contain
- * a hostname pattern that isn't inside a quantifier.
- */
-predicate alwaysMatchesHostname(RegExpTerm term) {
-  hasTopLevelDomainEnding(term, _)
-  or
-  // `localhost` is considered a hostname pattern, but has no TLD
-  term.(RegExpConstant).getValue().regexpMatch("\\blocalhost\\b")
-  or
-  not term instanceof RegExpAlt and
-  not term instanceof RegExpQuantifier and
-  alwaysMatchesHostname(term.getAChild())
-  or
-  alwaysMatchesHostnameAlt(term)
-}
-
-/** Holds if every child of `alt` contains a hostname pattern. */
-predicate alwaysMatchesHostnameAlt(RegExpAlt alt) {
-  alwaysMatchesHostnameAlt(alt, alt.getNumChild() - 1)
-}
-
-/**
- * Holds if the first `i` children of `alt` contains a hostname pattern.
- *
- * This is used instead of `forall` to avoid materializing the set of alternatives
- * that don't contains hostnames, which is much larger.
- */
-predicate alwaysMatchesHostnameAlt(RegExpAlt alt, int i) {
-  alwaysMatchesHostname(alt.getChild(0)) and i = 0
-  or
-  alwaysMatchesHostnameAlt(alt, i - 1) and
-  alwaysMatchesHostname(alt.getChild(i))
-}
-
-/**
- * Holds if `term` occurs inside a quantifier or alternative (and thus
- * can not be expected to correspond to a unique match), or as part of
- * a lookaround assertion (which are rarely used for capture groups).
- */
-predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
-  exists(RegExpParent parent | parent = term.getParent() |
-    parent instanceof RegExpAlt
-    or
-    parent instanceof RegExpQuantifier
-    or
-    parent instanceof RegExpSubPattern
-    or
-    isInsideChoiceOrSubPattern(parent)
-  )
-}
-
-/**
- * Holds if `group` is likely to be used as a capture group.
- */
-predicate isLikelyCaptureGroup(RegExpGroup group) {
-  group.isCapture() and
-  not isInsideChoiceOrSubPattern(group)
-}
-
-/**
- * Holds if `seq` contains two consecutive dots `..` or escaped dots.
- *
- * At least one of these dots is not intended to be a subdomain separator,
- * so we avoid flagging the pattern in this case.
- */
-predicate hasConsecutiveDots(RegExpSequence seq) {
-  exists(int i |
-    isDotLike(seq.getChild(i)) and
-    isDotLike(seq.getChild(i + 1))
-  )
-}
-
-predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
-  seq = regexp.getAChild*() and
-  exists(RegExpDot unescapedDot, int i, string hostname |
-    hasTopLevelDomainEnding(seq, i) and
-    not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
-    not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
-    unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
-    unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
-    not hasConsecutiveDots(unescapedDot.getParent()) and
-    hostname =
-      seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
-        seq.getChild(i).getRawValue()
-  |
-    if unescapedDot.getParent() instanceof RegExpQuantifier
-    then
-      // `.*\.example.com` can match `evil.com/?x=.example.com`
-      //
-      // This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
-      // We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
-      // Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
-      // and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
-      seq.getChild(0) instanceof RegExpCaret and
-      not seq.getAChild() instanceof RegExpDollar and
-      seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
-      msg =
-        "has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
-          + "' which may cause '" + hostname +
-          "' to be matched anywhere in the URL, outside the hostname."
-    else
-      msg =
-        "has an unescaped '.' before '" + hostname +
-          "', so it might match more hosts than expected."
-  )
-}
-
-predicate incompleteHostnameRegExp(
-  RegExpSequence hostSequence, string message, DataFlow::Node aux, string label
-) {
-  exists(RegExpPatternSource re, RegExpTerm regexp, string msg, string kind |
-    regexp = re.getRegExpTerm() and
-    isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
-    (
-      if re.getAParse() != re
-      then (
-        kind = "string, which is used as a regular expression $@," and
-        aux = re.getAParse()
-      ) else (
-        kind = "regular expression" and aux = re
-      )
-    )
-  |
-    message = "This " + kind + " " + msg and label = "here"
-  )
-}
diff --git a/python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll b/python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
deleted file mode 100644
index 2e13beb440b..00000000000
--- a/python/ql/src/Security/CWE-020/HostnameRegexpSpecific.qll
+++ /dev/null
@@ -1,2 +0,0 @@
-import semmle.python.security.performance.RegExpTreeView
-import semmle.python.dataflow.new.DataFlow
diff --git a/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
index db7129e5ac5..7f32419261c 100644
--- a/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
+++ b/python/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
@@ -8,9 +8,35 @@
  * @id py/incomplete-hostname-regexp
  * @tags correctness
  *       security
- *       external/cwe/cwe-020
+ *       external/cwe/cwe-20
  */
 
-import HostnameRegexpShared
+import python
+import semmle.python.regex
 
-query predicate problems = incompleteHostnameRegExp/4;
+private string commonTopLevelDomainRegex() { result = "com|org|edu|gov|uk|net|io" }
+
+/**
+ * Holds if `pattern` is a regular expression pattern for URLs with a host matched by `hostPart`,
+ * and `pattern` contains a subtle mistake that allows it to match unexpected hosts.
+ */
+bindingset[pattern]
+predicate isIncompleteHostNameRegExpPattern(string pattern, string hostPart) {
+  hostPart =
+    pattern
+        .regexpCapture("(?i).*" +
+            // an unescaped single `.`
+            "(?<!\\\\)[.]" +
+            // immediately followed by a sequence of subdomains, perhaps with some regex characters mixed in, followed by a known TLD
+            "([():|?a-z0-9-]+(\\\\)?[.](" + commonTopLevelDomainRegex() + "))" + ".*", 1)
+}
+
+from Regex r, string pattern, string hostPart
+where
+  r.getText() = pattern and
+  isIncompleteHostNameRegExpPattern(pattern, hostPart) and
+  // ignore patterns with capture groups after the TLD
+  not pattern.regexpMatch("(?i).*[.](" + commonTopLevelDomainRegex() + ").*[(][?]:.*[)].*")
+select r,
+  "This regular expression has an unescaped '.' before '" + hostPart +
+    "', so it might match more hosts than expected."
diff --git a/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
index 6a3c6769c35..00e0fcb95a0 100644
--- a/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
+++ b/python/ql/test/query-tests/Security/CWE-020-IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
@@ -1 +1 @@
-| hosttest.py:6:31:6:53 | (www\|beta).example.com/ | This regular expression has an unescaped '.' before 'example.com/', so it might match more hosts than expected. | hosttest.py:6:27:6:51 | ControlFlowNode for Str | here |
+| hosttest.py:6:27:6:51 | Str | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. |

From 4a279287289c6526092cc3f3404a4ec63002e5b4 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 18 Mar 2022 14:00:10 +0100
Subject: [PATCH 13/13] Ruby/JS add missing ^ in qhelp

---
 .../ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp      | 2 +-
 .../src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
index 86637d140df..3b49628c84f 100644
--- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
+++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp
@@ -59,7 +59,7 @@
 		<p>
 
 			Address this vulnerability by escaping <code>.</code>
-			appropriately: <code>let regex = /((www|beta)\.)?example\.com/</code>.
+			appropriately: <code>let regex = /^((www|beta)\.)?example\.com/</code>.
 
 		</p>
 
diff --git a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
index cb242aa8c61..44ec2175709 100644
--- a/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
+++ b/ruby/ql/src/queries/security/cwe-020/IncompleteHostnameRegExp.qhelp
@@ -59,7 +59,7 @@
 		<p>
 
 			Address this vulnerability by escaping <code>.</code>
-			appropriately: <code>regex = /((www|beta)\.)?example\.com/</code>.
+			appropriately: <code>regex = /^((www|beta)\.)?example\.com/</code>.
 
 		</p>