Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit 7c2358c

Browse files
authored
Merge pull request #181 from sauyon/reflectedxss-fps
ReflectedXSS refinement
2 parents 7af168f + ed87c34 commit 7c2358c

File tree

4 files changed

+86
-4
lines changed

4 files changed

+86
-4
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* The query "Reflected cross-site scripting" has been improved to more correctly determine whether
3+
an HTML mime type will be sniffed, which should lead to more accurate results.

ql/src/semmle/go/security/ReflectedXssCustomizations.qll

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,18 @@ module ReflectedXss {
6464
or
6565
exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") |
6666
body = call.getAnArgument() and
67-
// checks that the format value does not start with:
67+
// checks that the format value does not start with (ignoring whitespace as defined by
68+
// https://mimesniff.spec.whatwg.org/#whitespace-byte):
6869
// - '<', which could lead to an HTML content type being detected, or
6970
// - '%', which could be a format string.
70-
call.getArgument(1).getStringValue().regexpMatch("^[^<%].*")
71+
call.getArgument(1).getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<%].*")
7172
)
7273
or
7374
exists(DataFlow::Node pred | body = pred.getASuccessor*() |
74-
// data starting with a character other than `<` cannot cause an HTML content type to be detected.
75-
pred.getStringValue().regexpMatch("^[^<].*")
75+
// data starting with a character other than `<` (ignoring whitespace as defined by
76+
// https://mimesniff.spec.whatwg.org/#whitespace-byte) cannot cause an HTML content type to
77+
// be detected.
78+
pred.getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<].*")
7679
or
7780
// json data cannot begin with `<`
7881
exists(EncodingJson::MarshalFunction mf | pred = mf.getOutput().getNode(mf.getACall()))

ql/test/query-tests/Security/CWE-079/ReflectedXss.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ edges
22
| ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:44:14:51 | username |
33
| contenttype.go:11:11:11:16 | selection of Form : Values | contenttype.go:17:11:17:22 | type conversion |
44
| contenttype.go:49:11:49:16 | selection of Form : Values | contenttype.go:53:34:53:37 | data |
5+
| contenttype.go:63:11:63:29 | call to FormValue : string | contenttype.go:64:52:64:55 | data |
6+
| contenttype.go:73:11:73:29 | call to FormValue : string | contenttype.go:79:11:79:14 | data |
7+
| contenttype.go:88:11:88:29 | call to FormValue : string | contenttype.go:91:4:91:7 | data |
8+
| contenttype.go:113:11:113:29 | call to FormValue : string | contenttype.go:114:50:114:53 | data |
59
| tst.go:14:15:14:20 | selection of Form : Values | tst.go:18:12:18:39 | type conversion |
610
| tst.go:48:14:48:19 | selection of Form : Values | tst.go:53:12:53:26 | type conversion |
711
| websocketXss.go:30:7:30:10 | definition of xnet : slice type | websocketXss.go:32:24:32:27 | xnet |
@@ -17,6 +21,14 @@ nodes
1721
| contenttype.go:17:11:17:22 | type conversion | semmle.label | type conversion |
1822
| contenttype.go:49:11:49:16 | selection of Form : Values | semmle.label | selection of Form : Values |
1923
| contenttype.go:53:34:53:37 | data | semmle.label | data |
24+
| contenttype.go:63:11:63:29 | call to FormValue : string | semmle.label | call to FormValue : string |
25+
| contenttype.go:64:52:64:55 | data | semmle.label | data |
26+
| contenttype.go:73:11:73:29 | call to FormValue : string | semmle.label | call to FormValue : string |
27+
| contenttype.go:79:11:79:14 | data | semmle.label | data |
28+
| contenttype.go:88:11:88:29 | call to FormValue : string | semmle.label | call to FormValue : string |
29+
| contenttype.go:91:4:91:7 | data | semmle.label | data |
30+
| contenttype.go:113:11:113:29 | call to FormValue : string | semmle.label | call to FormValue : string |
31+
| contenttype.go:114:50:114:53 | data | semmle.label | data |
2032
| tst.go:14:15:14:20 | selection of Form : Values | semmle.label | selection of Form : Values |
2133
| tst.go:18:12:18:39 | type conversion | semmle.label | type conversion |
2234
| tst.go:48:14:48:19 | selection of Form : Values | semmle.label | selection of Form : Values |
@@ -37,6 +49,10 @@ nodes
3749
| ReflectedXss.go:14:44:14:51 | username | ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:44:14:51 | username | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:11:15:11:20 | selection of Form | user-provided value |
3850
| contenttype.go:17:11:17:22 | type conversion | contenttype.go:11:11:11:16 | selection of Form : Values | contenttype.go:17:11:17:22 | type conversion | Cross-site scripting vulnerability due to $@. | contenttype.go:11:11:11:16 | selection of Form | user-provided value |
3951
| contenttype.go:53:34:53:37 | data | contenttype.go:49:11:49:16 | selection of Form : Values | contenttype.go:53:34:53:37 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:49:11:49:16 | selection of Form | user-provided value |
52+
| contenttype.go:64:52:64:55 | data | contenttype.go:63:11:63:29 | call to FormValue : string | contenttype.go:64:52:64:55 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:63:11:63:29 | call to FormValue | user-provided value |
53+
| contenttype.go:79:11:79:14 | data | contenttype.go:73:11:73:29 | call to FormValue : string | contenttype.go:79:11:79:14 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:73:11:73:29 | call to FormValue | user-provided value |
54+
| contenttype.go:91:4:91:7 | data | contenttype.go:88:11:88:29 | call to FormValue : string | contenttype.go:91:4:91:7 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:88:11:88:29 | call to FormValue | user-provided value |
55+
| contenttype.go:114:50:114:53 | data | contenttype.go:113:11:113:29 | call to FormValue : string | contenttype.go:114:50:114:53 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:113:11:113:29 | call to FormValue | user-provided value |
4056
| tst.go:18:12:18:39 | type conversion | tst.go:14:15:14:20 | selection of Form : Values | tst.go:18:12:18:39 | type conversion | Cross-site scripting vulnerability due to $@. | tst.go:14:15:14:20 | selection of Form | user-provided value |
4157
| tst.go:53:12:53:26 | type conversion | tst.go:48:14:48:19 | selection of Form : Values | tst.go:53:12:53:26 | type conversion | Cross-site scripting vulnerability due to $@. | tst.go:48:14:48:19 | selection of Form | user-provided value |
4258
| websocketXss.go:32:24:32:27 | xnet | websocketXss.go:30:7:30:10 | definition of xnet : slice type | websocketXss.go:32:24:32:27 | xnet | Cross-site scripting vulnerability due to $@. | websocketXss.go:30:7:30:10 | definition of xnet | user-provided value |

ql/test/query-tests/Security/CWE-079/contenttype.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,63 @@ func serve5() {
5454
})
5555
http.ListenAndServe(":80", nil)
5656
}
57+
58+
func serve10() {
59+
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
60+
r.ParseForm()
61+
data := r.Form.Get("data")
62+
63+
data := r.FormValue("data")
64+
fmt.Fprintf(w, "\t<html><body>%s</body></html>", data) // Not OK
65+
})
66+
}
67+
68+
func serve11() {
69+
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
70+
r.ParseForm()
71+
data := r.Form.Get("data")
72+
73+
data := r.FormValue("data")
74+
fmt.Fprintf(w, `
75+
<html>
76+
<body>
77+
%s
78+
</body>
79+
</html>`, data) // Not OK
80+
})
81+
}
82+
83+
func serve12() {
84+
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
85+
r.ParseForm()
86+
data := r.Form.Get("data")
87+
88+
data := r.FormValue("data")
89+
fmt.Fprintf(w, `
90+
%s
91+
`, data) // Not OK
92+
})
93+
}
94+
95+
func serve13() {
96+
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
97+
r.ParseForm()
98+
data := r.Form.Get("data")
99+
100+
data := r.FormValue("data")
101+
fmt.Fprintf(w, `
102+
Echoed:
103+
%s
104+
`, data) // OK
105+
})
106+
}
107+
108+
func serve14() {
109+
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
110+
r.ParseForm()
111+
data := r.Form.Get("data")
112+
113+
data := r.FormValue("data")
114+
fmt.Fprintf(w, "<html><body>%s</body></html>", data) // Not OK
115+
})
116+
}

0 commit comments

Comments
 (0)