Skip to content

Commit 2959ed3

Browse files
committed
Go: Handle quoted dots in hostname regexps
1 parent 449d672 commit 2959ed3

4 files changed

Lines changed: 171 additions & 5 deletions

File tree

‎go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,25 @@
1414

1515
import go
1616

17+
/** Escapes dots within quoted sections for the hostname pattern check. */
18+
bindingset[pattern]
19+
private string escapeQuotedDots(string pattern) {
20+
if not pattern.matches("%\\Q%")
21+
then result = pattern
22+
else
23+
result =
24+
concat(string part, string escapedPart, int i |
25+
// Consume quoted sections before other escapes. Outside quoting, paired
26+
// backslashes cannot open a section; inside, the first \E closes it.
27+
part = pattern.regexpFind("\\\\Q[\\s\\S]*?(?:\\\\E|\\z)|\\\\[^Q]|[^\\\\]+|\\\\\\z", i, _) and
28+
if part.prefix(2) = "\\Q"
29+
then escapedPart = part.replaceAll(".", "\\.")
30+
else escapedPart = part
31+
|
32+
escapedPart, "" order by i
33+
)
34+
}
35+
1736
/**
1837
* Holds if `pattern` is a regular expression pattern for URLs with a host matched by `hostPart`,
1938
* and `pattern` contains a subtle mistake that allows it to match unexpected hosts.
@@ -114,7 +133,21 @@ import Flow::PathGraph
114133
from Flow::PathNode source, Flow::PathNode sink, string hostPart
115134
where
116135
Flow::flowPath(source, sink) and
117-
IncompleteHostNameRegexpConfig::isSourceString(source.getNode(), hostPart)
136+
// A preceding concatenation can escape a quote opener in a source fragment.
137+
// Interpret quoting only when the complete pattern is known.
138+
if
139+
exists(string pattern |
140+
pattern = sink.getNode().(RegexpPattern).getPattern() and
141+
pattern.matches("%\\Q%")
142+
)
143+
then
144+
exists(string pattern |
145+
pattern = sink.getNode().(RegexpPattern).getPattern() and
146+
pattern = source.getNode().asExpr().getStringValue()
147+
|
148+
isIncompleteHostNameRegexpPattern(escapeQuotedDots(pattern), hostPart)
149+
)
150+
else IncompleteHostNameRegexpConfig::isSourceString(source.getNode(), hostPart)
118151
select source, source, sink,
119152
"This regular expression has an unescaped dot before '" + hostPart + "', " +
120153
"so it might match more hosts than expected when $@.", sink, "the regular expression is used"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `go/incomplete-hostname-regexp` query now recognizes `\Q...\E` quoted text in constant regular expression patterns, avoiding false positives for literal hostname dots.

‎go/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexp.expected‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,37 @@
55
| main.go:49:21:49:45 | `https://www.example.com` | main.go:49:21:49:45 | `https://www.example.com` | main.go:65:15:65:23 | localVar3 | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:65:15:65:23 | localVar3 | the regular expression is used |
66
| main.go:56:15:56:34 | ...+... | main.go:56:15:56:34 | ...+... | main.go:56:15:56:34 | ...+... | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:56:15:56:34 | ...+... | the regular expression is used |
77
| main.go:58:15:58:42 | ...+... | main.go:58:15:58:42 | ...+... | main.go:58:15:58:42 | ...+... | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:58:15:58:42 | ...+... | the regular expression is used |
8+
| quoted-hostnames.go:10:21:10:43 | `^https://github.com/$` | quoted-hostnames.go:10:21:10:43 | `^https://github.com/$` | quoted-hostnames.go:10:21:10:43 | `^https://github.com/$` | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:10:21:10:43 | `^https://github.com/$` | the regular expression is used |
9+
| quoted-hostnames.go:16:21:16:47 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:16:21:16:47 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:16:21:16:47 | `^https://\\Qgithub\\E.com/$` | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:16:21:16:47 | `^https://\\Qgithub\\E.com/$` | the regular expression is used |
10+
| quoted-hostnames.go:19:21:19:72 | `^(?:\\Qhttps://github.com/\\E\|https://example.org/)$` | quoted-hostnames.go:19:21:19:72 | `^(?:\\Qhttps://github.com/\\E\|https://example.org/)$` | quoted-hostnames.go:19:21:19:72 | `^(?:\\Qhttps://github.com/\\E\|https://example.org/)$` | This regular expression has an unescaped dot before 'org', so it might match more hosts than expected when $@. | quoted-hostnames.go:19:21:19:72 | `^(?:\\Qhttps://github.com/\\E\|https://example.org/)$` | the regular expression is used |
11+
| quoted-hostnames.go:28:21:28:46 | `^\\\\Qhttps://github.com/$` | quoted-hostnames.go:28:21:28:46 | `^\\\\Qhttps://github.com/$` | quoted-hostnames.go:28:21:28:46 | `^\\\\Qhttps://github.com/$` | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:28:21:28:46 | `^\\\\Qhttps://github.com/$` | the regular expression is used |
12+
| quoted-hostnames.go:34:21:34:74 | `^(?:\\Qhttps://github\\Q.com/\\E\|https://example.org/)$` | quoted-hostnames.go:34:21:34:74 | `^(?:\\Qhttps://github\\Q.com/\\E\|https://example.org/)$` | quoted-hostnames.go:34:21:34:74 | `^(?:\\Qhttps://github\\Q.com/\\E\|https://example.org/)$` | This regular expression has an unescaped dot before 'org', so it might match more hosts than expected when $@. | quoted-hostnames.go:34:21:34:74 | `^(?:\\Qhttps://github\\Q.com/\\E\|https://example.org/)$` | the regular expression is used |
13+
| quoted-hostnames.go:37:21:37:48 | `^\\Qhttps://github\\\\E.com/$` | quoted-hostnames.go:37:21:37:48 | `^\\Qhttps://github\\\\E.com/$` | quoted-hostnames.go:37:21:37:48 | `^\\Qhttps://github\\\\E.com/$` | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:37:21:37:48 | `^\\Qhttps://github\\\\E.com/$` | the regular expression is used |
14+
| quoted-hostnames.go:46:21:46:49 | "^https://\\\\Qgithub\\\\E.com/$" | quoted-hostnames.go:46:21:46:49 | "^https://\\\\Qgithub\\\\E.com/$" | quoted-hostnames.go:46:21:46:49 | "^https://\\\\Qgithub\\\\E.com/$" | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:46:21:46:49 | "^https://\\\\Qgithub\\\\E.com/$" | the regular expression is used |
15+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:61:21:61:38 | wildcardAfterQuote | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:61:21:61:38 | wildcardAfterQuote | the regular expression is used |
16+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:62:17:62:34 | wildcardAfterQuote | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:62:17:62:34 | wildcardAfterQuote | the regular expression is used |
17+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:63:21:63:38 | wildcardAfterQuote | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:63:21:63:38 | wildcardAfterQuote | the regular expression is used |
18+
| quoted-hostnames.go:69:12:69:37 | `\\Q)?https://github.com/$` | quoted-hostnames.go:69:12:69:37 | `\\Q)?https://github.com/$` | quoted-hostnames.go:70:21:70:35 | ...+... | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:70:21:70:35 | ...+... | the regular expression is used |
19+
| quoted-hostnames.go:73:30:73:55 | `\\Q)?https://github.com/$` | quoted-hostnames.go:73:30:73:55 | `\\Q)?https://github.com/$` | quoted-hostnames.go:73:21:73:55 | ...+... | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:73:21:73:55 | ...+... | the regular expression is used |
20+
| quoted-hostnames.go:76:21:76:56 | ...+... | quoted-hostnames.go:76:21:76:56 | ...+... | quoted-hostnames.go:76:21:76:56 | ...+... | This regular expression has an unescaped dot before 'com', so it might match more hosts than expected when $@. | quoted-hostnames.go:76:21:76:56 | ...+... | the regular expression is used |
821
edges
9-
| IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | IncompleteHostnameRegexp.go:12:38:12:39 | re | provenance | Sink:MaD:2 |
22+
| IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | IncompleteHostnameRegexp.go:12:38:12:39 | re | provenance | Sink:MaD:3 |
1023
| main.go:49:21:49:45 | `https://www.example.com` | main.go:62:15:62:25 | sourceConst | provenance | |
11-
| main.go:62:15:62:25 | sourceConst | main.go:65:15:65:23 | localVar3 | provenance | Sink:MaD:1 |
24+
| main.go:62:15:62:25 | sourceConst | main.go:65:15:65:23 | localVar3 | provenance | Sink:MaD:2 |
25+
| quoted-hostnames.go:52:24:52:50 | `^\\Qhttps://github.com/\\E$` | quoted-hostnames.go:57:21:57:34 | quotedHostname | provenance | Sink:MaD:4 |
26+
| quoted-hostnames.go:52:24:52:50 | `^\\Qhttps://github.com/\\E$` | quoted-hostnames.go:58:17:58:30 | quotedHostname | provenance | Sink:MaD:1 |
27+
| quoted-hostnames.go:52:24:52:50 | `^\\Qhttps://github.com/\\E$` | quoted-hostnames.go:59:21:59:34 | quotedHostname | provenance | Sink:MaD:3 |
28+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:61:21:61:38 | wildcardAfterQuote | provenance | Sink:MaD:4 |
29+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:62:17:62:34 | wildcardAfterQuote | provenance | Sink:MaD:1 |
30+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | quoted-hostnames.go:63:21:63:38 | wildcardAfterQuote | provenance | Sink:MaD:3 |
31+
| quoted-hostnames.go:69:12:69:37 | `\\Q)?https://github.com/$` | quoted-hostnames.go:70:30:70:35 | suffix | provenance | |
32+
| quoted-hostnames.go:70:30:70:35 | suffix | quoted-hostnames.go:70:21:70:35 | ...+... | provenance | Config Sink:MaD:4 |
33+
| quoted-hostnames.go:73:30:73:55 | `\\Q)?https://github.com/$` | quoted-hostnames.go:73:21:73:55 | ...+... | provenance | Config Sink:MaD:4 |
1234
models
13-
| 1 | Sink: regexp; ; false; Match; ; ; Argument[0]; regex-use[1]; manual |
14-
| 2 | Sink: regexp; ; false; MatchString; ; ; Argument[0]; regex-use[1]; manual |
35+
| 1 | Sink: regexp; ; false; Compile; ; ; Argument[0]; regex-use[c]; manual |
36+
| 2 | Sink: regexp; ; false; Match; ; ; Argument[0]; regex-use[1]; manual |
37+
| 3 | Sink: regexp; ; false; MatchString; ; ; Argument[0]; regex-use[1]; manual |
38+
| 4 | Sink: regexp; ; false; MustCompile; ; ; Argument[0]; regex-use[c]; manual |
1539
nodes
1640
| IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | semmle.label | "^((www\|beta).)?example.com/" |
1741
| IncompleteHostnameRegexp.go:12:38:12:39 | re | semmle.label | re |
@@ -22,4 +46,32 @@ nodes
2246
| main.go:58:15:58:42 | ...+... | semmle.label | ...+... |
2347
| main.go:62:15:62:25 | sourceConst | semmle.label | sourceConst |
2448
| main.go:65:15:65:23 | localVar3 | semmle.label | localVar3 |
49+
| quoted-hostnames.go:7:21:7:87 | `^// TODO (\\Qhttps://github.com/FerretDB/\\E([-\\w]+)/issues/(\\d+))$` | semmle.label | `^// TODO (\\Qhttps://github.com/FerretDB/\\E([-\\w]+)/issues/(\\d+))$` |
50+
| quoted-hostnames.go:10:21:10:43 | `^https://github.com/$` | semmle.label | `^https://github.com/$` |
51+
| quoted-hostnames.go:16:21:16:47 | `^https://\\Qgithub\\E.com/$` | semmle.label | `^https://\\Qgithub\\E.com/$` |
52+
| quoted-hostnames.go:19:21:19:72 | `^(?:\\Qhttps://github.com/\\E\|https://example.org/)$` | semmle.label | `^(?:\\Qhttps://github.com/\\E\|https://example.org/)$` |
53+
| quoted-hostnames.go:22:21:22:76 | `^(?:\\Qhttps://github.com/\\E\|\\Qhttps://example.org/\\E)$` | semmle.label | `^(?:\\Qhttps://github.com/\\E\|\\Qhttps://example.org/\\E)$` |
54+
| quoted-hostnames.go:25:21:25:44 | `^\\Qhttps://github.com/` | semmle.label | `^\\Qhttps://github.com/` |
55+
| quoted-hostnames.go:28:21:28:46 | `^\\\\Qhttps://github.com/$` | semmle.label | `^\\\\Qhttps://github.com/$` |
56+
| quoted-hostnames.go:31:21:31:49 | `^\\\\\\Qhttps://github.com/\\E$` | semmle.label | `^\\\\\\Qhttps://github.com/\\E$` |
57+
| quoted-hostnames.go:34:21:34:74 | `^(?:\\Qhttps://github\\Q.com/\\E\|https://example.org/)$` | semmle.label | `^(?:\\Qhttps://github\\Q.com/\\E\|https://example.org/)$` |
58+
| quoted-hostnames.go:37:21:37:48 | `^\\Qhttps://github\\\\E.com/$` | semmle.label | `^\\Qhttps://github\\\\E.com/$` |
59+
| quoted-hostnames.go:40:21:40:66 | `^\\Qhttps://github.com/\\ehttps://example.org/` | semmle.label | `^\\Qhttps://github.com/\\ehttps://example.org/` |
60+
| quoted-hostnames.go:43:21:43:49 | "^\\\\Qhttps://github.com/\\\\E$" | semmle.label | "^\\\\Qhttps://github.com/\\\\E$" |
61+
| quoted-hostnames.go:46:21:46:49 | "^https://\\\\Qgithub\\\\E.com/$" | semmle.label | "^https://\\\\Qgithub\\\\E.com/$" |
62+
| quoted-hostnames.go:49:21:49:63 | `^\\Qhttps://github.com/\\E\\Qexample.org/\\E$` | semmle.label | `^\\Qhttps://github.com/\\E\\Qexample.org/\\E$` |
63+
| quoted-hostnames.go:52:24:52:50 | `^\\Qhttps://github.com/\\E$` | semmle.label | `^\\Qhttps://github.com/\\E$` |
64+
| quoted-hostnames.go:53:28:53:54 | `^https://\\Qgithub\\E.com/$` | semmle.label | `^https://\\Qgithub\\E.com/$` |
65+
| quoted-hostnames.go:57:21:57:34 | quotedHostname | semmle.label | quotedHostname |
66+
| quoted-hostnames.go:58:17:58:30 | quotedHostname | semmle.label | quotedHostname |
67+
| quoted-hostnames.go:59:21:59:34 | quotedHostname | semmle.label | quotedHostname |
68+
| quoted-hostnames.go:61:21:61:38 | wildcardAfterQuote | semmle.label | wildcardAfterQuote |
69+
| quoted-hostnames.go:62:17:62:34 | wildcardAfterQuote | semmle.label | wildcardAfterQuote |
70+
| quoted-hostnames.go:63:21:63:38 | wildcardAfterQuote | semmle.label | wildcardAfterQuote |
71+
| quoted-hostnames.go:69:12:69:37 | `\\Q)?https://github.com/$` | semmle.label | `\\Q)?https://github.com/$` |
72+
| quoted-hostnames.go:70:21:70:35 | ...+... | semmle.label | ...+... |
73+
| quoted-hostnames.go:70:30:70:35 | suffix | semmle.label | suffix |
74+
| quoted-hostnames.go:73:21:73:55 | ...+... | semmle.label | ...+... |
75+
| quoted-hostnames.go:73:30:73:55 | `\\Q)?https://github.com/$` | semmle.label | `\\Q)?https://github.com/$` |
76+
| quoted-hostnames.go:76:21:76:56 | ...+... | semmle.label | ...+... |
2577
subpaths
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package main
2+
3+
import "regexp"
4+
5+
func quotedHostnames() {
6+
// The hostname in the original issue is literal text.
7+
regexp.MustCompile(`^// TODO (\Qhttps://github.com/FerretDB/\E([-\w]+)/issues/(\d+))$`)
8+
9+
// An ordinary wildcard dot remains unsafe.
10+
regexp.MustCompile(`^https://github.com/$`) // $ Alert
11+
12+
// An explicitly escaped dot is literal.
13+
regexp.MustCompile(`^https://github\.com/$`)
14+
15+
// A dot after the quoted region remains a wildcard.
16+
regexp.MustCompile(`^https://\Qgithub\E.com/$`) // $ Alert
17+
18+
// A quoted hostname must not hide a later unsafe alternative.
19+
regexp.MustCompile(`^(?:\Qhttps://github.com/\E|https://example.org/)$`) // $ Alert
20+
21+
// Both alternatives are literal, in separate quoted regions.
22+
regexp.MustCompile(`^(?:\Qhttps://github.com/\E|\Qhttps://example.org/\E)$`)
23+
24+
// An unterminated quoted region extends to the end of the pattern.
25+
regexp.MustCompile(`^\Qhttps://github.com/`)
26+
27+
// Two backslashes escape the opener, leaving the dot as a wildcard.
28+
regexp.MustCompile(`^\\Qhttps://github.com/$`) // $ Alert
29+
30+
// Three backslashes leave a real opener after a literal backslash.
31+
regexp.MustCompile(`^\\\Qhttps://github.com/\E$`)
32+
33+
// Another opener is literal text; the first closing marker still ends quoting.
34+
regexp.MustCompile(`^(?:\Qhttps://github\Q.com/\E|https://example.org/)$`) // $ Alert
35+
36+
// The first literal closing marker ends quoting even after another backslash.
37+
regexp.MustCompile(`^\Qhttps://github\\E.com/$`) // $ Alert
38+
39+
// A lowercase closing marker is literal and leaves the region open.
40+
regexp.MustCompile(`^\Qhttps://github.com/\ehttps://example.org/`)
41+
42+
// Interpreted Go strings decode to the same quoted pattern.
43+
regexp.MustCompile("^\\Qhttps://github.com/\\E$")
44+
45+
// Decoding an interpreted string does not quote the following wildcard.
46+
regexp.MustCompile("^https://\\Qgithub\\E.com/$") // $ Alert
47+
48+
// Adjacent quoted sections remain literal.
49+
regexp.MustCompile(`^\Qhttps://github.com/\E\Qexample.org/\E$`)
50+
}
51+
52+
const quotedHostname = `^\Qhttps://github.com/\E$`
53+
const wildcardAfterQuote = `^https://\Qgithub\E.com/$` // $ Alert
54+
55+
func constantQuotePatterns() {
56+
// Constant aliases supply complete patterns to compilation and matching APIs.
57+
regexp.MustCompile(quotedHostname)
58+
regexp.Compile(quotedHostname)
59+
regexp.MatchString(quotedHostname, "https://github.com/")
60+
61+
regexp.MustCompile(wildcardAfterQuote) // $ Sink
62+
regexp.Compile(wildcardAfterQuote) // $ Sink
63+
regexp.MatchString(wildcardAfterQuote, "https://github.com/") // $ Sink
64+
}
65+
66+
func concatenatedQuoteOpeners() {
67+
// The prefix escapes the suffix's opener, leaving its hostname dot unsafe.
68+
prefix := `^(?:\`
69+
suffix := `\Q)?https://github.com/$` // $ Alert
70+
regexp.MustCompile(prefix + suffix) // $ Sink
71+
72+
// The same context matters when the suffix is inline.
73+
regexp.MustCompile(prefix + `\Q)?https://github.com/$`) // $ Alert
74+
75+
// A constant concatenation supplies the complete pattern.
76+
regexp.MustCompile(`^(?:\` + `\Q)?https://github.com/$`) // $ Alert
77+
}

0 commit comments

Comments
 (0)