Skip to content

Commit 0bc652c

Browse files
owen-mcowen-mc
committed
Fix test for reflected xss sanitizer
It used to pass even without ErrorSanitizer because `cookie` is already sanitized.
1 parent 4dd1ed8 commit 0bc652c

File tree

2 files changed

+16
-14
lines changed

2 files changed

+16
-14
lines changed

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
| contenttype.go:79:11:79:14 | data | contenttype.go:73:10:73:28 | call to FormValue | contenttype.go:79:11:79:14 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:73:10:73:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | |
77
| contenttype.go:91:4:91:7 | data | contenttype.go:88:10:88:28 | call to FormValue | contenttype.go:91:4:91:7 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:88:10:88:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | |
88
| contenttype.go:114:50:114:53 | data | contenttype.go:113:10:113:28 | call to FormValue | contenttype.go:114:50:114:53 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:113:10:113:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | |
9-
| reflectedxsstest.go:33:10:33:57 | type conversion | reflectedxsstest.go:31:2:31:44 | ... := ...[0] | reflectedxsstest.go:33:10:33:57 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:31:2:31:44 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
10-
| reflectedxsstest.go:34:10:34:62 | type conversion | reflectedxsstest.go:31:2:31:44 | ... := ...[1] | reflectedxsstest.go:34:10:34:62 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:31:2:31:44 | ... := ...[1] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
9+
| reflectedxsstest.go:33:10:33:57 | type conversion | reflectedxsstest.go:30:2:30:44 | ... := ...[0] | reflectedxsstest.go:33:10:33:57 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:30:2:30:44 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
10+
| reflectedxsstest.go:34:10:34:62 | type conversion | reflectedxsstest.go:30:2:30:44 | ... := ...[1] | reflectedxsstest.go:34:10:34:62 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:30:2:30:44 | ... := ...[1] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
1111
| reflectedxsstest.go:44:10:44:55 | type conversion | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | reflectedxsstest.go:44:10:44:55 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
1212
| reflectedxsstest.go:45:10:45:18 | byteSlice | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | reflectedxsstest.go:45:10:45:18 | byteSlice | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
1313
| reflectedxsstest.go:54:11:54:21 | type conversion | reflectedxsstest.go:51:14:51:18 | selection of URL | reflectedxsstest.go:54:11:54:21 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:51:14:51:18 | selection of URL | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | |
@@ -30,10 +30,11 @@ edges
3030
| contenttype.go:73:10:73:28 | call to FormValue | contenttype.go:79:11:79:14 | data | provenance | Src:MaD:8 |
3131
| contenttype.go:88:10:88:28 | call to FormValue | contenttype.go:91:4:91:7 | data | provenance | Src:MaD:8 |
3232
| contenttype.go:113:10:113:28 | call to FormValue | contenttype.go:114:50:114:53 | data | provenance | Src:MaD:8 |
33-
| reflectedxsstest.go:31:2:31:44 | ... := ...[0] | reflectedxsstest.go:32:30:32:33 | file | provenance | Src:MaD:7 |
34-
| reflectedxsstest.go:31:2:31:44 | ... := ...[1] | reflectedxsstest.go:34:46:34:60 | selection of Filename | provenance | Src:MaD:7 |
35-
| reflectedxsstest.go:32:2:32:34 | ... := ...[0] | reflectedxsstest.go:33:49:33:55 | content | provenance | |
36-
| reflectedxsstest.go:32:30:32:33 | file | reflectedxsstest.go:32:2:32:34 | ... := ...[0] | provenance | MaD:13 |
33+
| reflectedxsstest.go:30:2:30:44 | ... := ...[0] | reflectedxsstest.go:31:30:31:33 | file | provenance | Src:MaD:7 |
34+
| reflectedxsstest.go:30:2:30:44 | ... := ...[1] | reflectedxsstest.go:34:46:34:60 | selection of Filename | provenance | Src:MaD:7 |
35+
| reflectedxsstest.go:31:2:31:34 | ... := ...[0] | reflectedxsstest.go:32:48:32:54 | content | provenance | |
36+
| reflectedxsstest.go:31:30:31:33 | file | reflectedxsstest.go:31:2:31:34 | ... := ...[0] | provenance | MaD:13 |
37+
| reflectedxsstest.go:32:48:32:54 | content | reflectedxsstest.go:33:49:33:55 | content | provenance | |
3738
| reflectedxsstest.go:33:17:33:56 | []type{args} [array] | reflectedxsstest.go:33:17:33:56 | call to Sprintf | provenance | MaD:12 |
3839
| reflectedxsstest.go:33:17:33:56 | call to Sprintf | reflectedxsstest.go:33:10:33:57 | type conversion | provenance | |
3940
| reflectedxsstest.go:33:49:33:55 | content | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | provenance | |
@@ -106,10 +107,11 @@ nodes
106107
| contenttype.go:91:4:91:7 | data | semmle.label | data |
107108
| contenttype.go:113:10:113:28 | call to FormValue | semmle.label | call to FormValue |
108109
| contenttype.go:114:50:114:53 | data | semmle.label | data |
109-
| reflectedxsstest.go:31:2:31:44 | ... := ...[0] | semmle.label | ... := ...[0] |
110-
| reflectedxsstest.go:31:2:31:44 | ... := ...[1] | semmle.label | ... := ...[1] |
111-
| reflectedxsstest.go:32:2:32:34 | ... := ...[0] | semmle.label | ... := ...[0] |
112-
| reflectedxsstest.go:32:30:32:33 | file | semmle.label | file |
110+
| reflectedxsstest.go:30:2:30:44 | ... := ...[0] | semmle.label | ... := ...[0] |
111+
| reflectedxsstest.go:30:2:30:44 | ... := ...[1] | semmle.label | ... := ...[1] |
112+
| reflectedxsstest.go:31:2:31:34 | ... := ...[0] | semmle.label | ... := ...[0] |
113+
| reflectedxsstest.go:31:30:31:33 | file | semmle.label | file |
114+
| reflectedxsstest.go:32:48:32:54 | content | semmle.label | content |
113115
| reflectedxsstest.go:33:10:33:57 | type conversion | semmle.label | type conversion |
114116
| reflectedxsstest.go:33:17:33:56 | []type{args} [array] | semmle.label | []type{args} [array] |
115117
| reflectedxsstest.go:33:17:33:56 | call to Sprintf | semmle.label | call to Sprintf |

‎go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ func ServeJsonDirect(w http.ResponseWriter, r http.Request) {
2525

2626
func ErrTest(w http.ResponseWriter, r http.Request) {
2727
cookie, err := r.Cookie("somecookie")
28-
w.Write([]byte(fmt.Sprintf("Cookie result: %v", cookie))) // GOOD: Cookie's value is not user-controlled in reflected xss.
29-
w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless
30-
http.Error(w, fmt.Sprintf("Cookie result: %v", cookie), 500) // Good: only plain text is written.
31-
file, header, err := r.FormFile("someFile") // $ Source[go/reflected-xss]
28+
w.Write([]byte(fmt.Sprintf("Cookie result: %v", cookie))) // GOOD: Cookie's value is not user-controlled in reflected xss.
29+
w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless
30+
file, header, err := r.FormFile("someFile") // $ Source[go/reflected-xss]
3231
content, err2 := io.ReadAll(file)
32+
http.Error(w, fmt.Sprintf("File content: %v", content), 500) // Good: only plain text is written.
3333
w.Write([]byte(fmt.Sprintf("File content: %v", content))) // $ Alert[go/reflected-xss] // BAD: file content is user-controlled
3434
w.Write([]byte(fmt.Sprintf("File name: %v", header.Filename))) // $ Alert[go/reflected-xss] // BAD: file header is user-controlled
3535
w.Write([]byte(fmt.Sprintf("FormFile error: %v", err))) // GOOD: FormFile's err return is harmless

0 commit comments

Comments
 (0)