Skip to content

Commit ef5e605

Browse files
owen-mcowen-mc
authored
Merge pull request #19386 from owen-mc/go/promote/html-template-escaping-bypass-xss
Go: promote `html-template-escaping-bypass-xss`
2 parents 5bfed77 + c933ab4 commit ef5e605

32 files changed

+308
-409
lines changed

‎go/ql/integration-tests/query-suite/go-code-scanning.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ ql/go/ql/src/Security/CWE-022/TaintedPath.ql
88
ql/go/ql/src/Security/CWE-022/UnsafeUnzipSymlink.ql
99
ql/go/ql/src/Security/CWE-022/ZipSlip.ql
1010
ql/go/ql/src/Security/CWE-078/CommandInjection.ql
11+
ql/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql
1112
ql/go/ql/src/Security/CWE-079/ReflectedXss.ql
1213
ql/go/ql/src/Security/CWE-089/SqlInjection.ql
1314
ql/go/ql/src/Security/CWE-089/StringBreak.ql

‎go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ ql/go/ql/src/Security/CWE-022/TaintedPath.ql
3030
ql/go/ql/src/Security/CWE-022/UnsafeUnzipSymlink.ql
3131
ql/go/ql/src/Security/CWE-022/ZipSlip.ql
3232
ql/go/ql/src/Security/CWE-078/CommandInjection.ql
33+
ql/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql
3334
ql/go/ql/src/Security/CWE-079/ReflectedXss.ql
3435
ql/go/ql/src/Security/CWE-089/SqlInjection.ql
3536
ql/go/ql/src/Security/CWE-089/StringBreak.ql

‎go/ql/integration-tests/query-suite/go-security-extended.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ ql/go/ql/src/Security/CWE-022/TaintedPath.ql
88
ql/go/ql/src/Security/CWE-022/UnsafeUnzipSymlink.ql
99
ql/go/ql/src/Security/CWE-022/ZipSlip.ql
1010
ql/go/ql/src/Security/CWE-078/CommandInjection.ql
11+
ql/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql
1112
ql/go/ql/src/Security/CWE-079/ReflectedXss.ql
1213
ql/go/ql/src/Security/CWE-089/SqlInjection.ql
1314
ql/go/ql/src/Security/CWE-089/StringBreak.ql

‎go/ql/integration-tests/query-suite/not_included_in_qls.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ ql/go/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql
2121
ql/go/ql/src/experimental/CWE-525/WebCacheDeception.ql
2222
ql/go/ql/src/experimental/CWE-74/DsnInjection.ql
2323
ql/go/ql/src/experimental/CWE-74/DsnInjectionLocal.ql
24-
ql/go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.ql
2524
ql/go/ql/src/experimental/CWE-807/SensitiveConditionBypass.ql
2625
ql/go/ql/src/experimental/CWE-840/ConditionalBypass.ql
2726
ql/go/ql/src/experimental/CWE-918/SSRF.ql

‎go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.qhelp renamed to ‎go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qhelp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
that allow values to be rendered as-is in the template, avoiding the escaping that all the other strings go
99
through.
1010
</p>
11-
<p>Using them on user-provided values will result in an opportunity for XSS.</p>
11+
<p>Using them on user-provided values allows for a cross-site scripting vulnerability.</p>
1212
</overview>
1313
<recommendation>
1414
<p>
@@ -19,10 +19,10 @@
1919
<p>
2020
In the first example you can see the special types and how they are used in a template:
2121
</p>
22-
<sample src="HTMLTemplateEscapingPassthroughBad.go" />
22+
<sample src="HtmlTemplateEscapingBypassXssBad.go" />
2323
<p>
2424
To avoid XSS, all user input should be a normal string type.
2525
</p>
26-
<sample src="HTMLTemplateEscapingPassthroughGood.go" />
26+
<sample src="HtmlTemplateEscapingBypassXssGood.go" />
2727
</example>
2828
</qhelp>

‎go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* @name Cross-site scripting via HTML template escaping bypass
3+
* @description Converting user input to a special type that avoids escaping
4+
* when fed into an HTML template allows for a cross-site
5+
* scripting vulnerability.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @security-severity 6.1
9+
* @precision high
10+
* @id go/html-template-escaping-bypass-xss
11+
* @tags security
12+
* external/cwe/cwe-079
13+
* external/cwe/cwe-116
14+
*/
15+
16+
import go
17+
18+
/**
19+
* A type that will not be escaped when passed to a `html/template` template.
20+
*/
21+
class UnescapedType extends Type {
22+
UnescapedType() {
23+
this.hasQualifiedName("html/template",
24+
["CSS", "HTML", "HTMLAttr", "JS", "JSStr", "Srcset", "URL"])
25+
}
26+
}
27+
28+
/**
29+
* Holds if the sink is a data value argument of a template execution call.
30+
*
31+
* Note that this is slightly more general than
32+
* `SharedXss::HtmlTemplateSanitizer` because it uses `Function.getACall()`,
33+
* which finds calls through interfaces which the receiver implements. This
34+
* finds more results in practice.
35+
*/
36+
predicate isSinkToTemplateExec(DataFlow::Node sink) {
37+
exists(Method fn, string methodName, DataFlow::CallNode call |
38+
fn.hasQualifiedName("html/template", "Template", methodName) and
39+
call = fn.getACall()
40+
|
41+
methodName = "Execute" and sink = call.getArgument(1)
42+
or
43+
methodName = "ExecuteTemplate" and sink = call.getArgument(2)
44+
)
45+
}
46+
47+
/**
48+
* Data flow configuration that tracks flows from untrusted sources to template execution calls
49+
* which go through a conversion to an unescaped type.
50+
*/
51+
module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateConfigSig {
52+
private newtype TConversionState =
53+
TUnconverted() or
54+
TConverted(UnescapedType unescapedType)
55+
56+
/**
57+
* The flow state for tracking whether a conversion to an unescaped type has
58+
* occurred.
59+
*/
60+
class FlowState extends TConversionState {
61+
predicate isBeforeConversion() { this instanceof TUnconverted }
62+
63+
predicate isAfterConversion(UnescapedType unescapedType) { this = TConverted(unescapedType) }
64+
65+
/** Gets a textual representation of this element. */
66+
string toString() {
67+
this.isBeforeConversion() and result = "Unconverted"
68+
or
69+
exists(UnescapedType unescapedType | this.isAfterConversion(unescapedType) |
70+
result = "Converted to " + unescapedType.getQualifiedName()
71+
)
72+
}
73+
}
74+
75+
predicate isSource(DataFlow::Node source, FlowState state) {
76+
state.isBeforeConversion() and source instanceof ActiveThreatModelSource
77+
}
78+
79+
predicate isSink(DataFlow::Node sink, FlowState state) {
80+
state.isAfterConversion(_) and isSinkToTemplateExec(sink)
81+
}
82+
83+
predicate isBarrier(DataFlow::Node node) {
84+
node instanceof SharedXss::Sanitizer and not node instanceof SharedXss::HtmlTemplateSanitizer
85+
or
86+
node.getType() instanceof NumericType
87+
}
88+
89+
/**
90+
* When a conversion to a passthrough type is encountered, transition the flow state.
91+
*/
92+
predicate isAdditionalFlowStep(
93+
DataFlow::Node pred, FlowState predState, DataFlow::Node succ, FlowState succState
94+
) {
95+
exists(ConversionExpr conversion, UnescapedType unescapedType |
96+
// If not yet converted, look for a conversion to a passthrough type
97+
predState.isBeforeConversion() and
98+
succState.isAfterConversion(unescapedType) and
99+
succ.(DataFlow::TypeCastNode).getExpr() = conversion and
100+
pred.asExpr() = conversion.getOperand() and
101+
conversion.getType().getUnderlyingType*() = unescapedType
102+
)
103+
}
104+
}
105+
106+
module UntrustedToTemplateExecWithConversionFlow =
107+
TaintTracking::GlobalWithState<UntrustedToTemplateExecWithConversionConfig>;
108+
109+
import UntrustedToTemplateExecWithConversionFlow::PathGraph
110+
111+
from
112+
UntrustedToTemplateExecWithConversionFlow::PathNode untrustedSource,
113+
UntrustedToTemplateExecWithConversionFlow::PathNode templateExecCall, UnescapedType unescapedType
114+
where
115+
UntrustedToTemplateExecWithConversionFlow::flowPath(untrustedSource, templateExecCall) and
116+
templateExecCall.getState().isAfterConversion(unescapedType)
117+
select templateExecCall.getNode(), untrustedSource, templateExecCall,
118+
"Data from an $@ will not be auto-escaped because it was converted to template." +
119+
unescapedType.getName(), untrustedSource.getNode(), "untrusted source"

‎go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssBad.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package main
2+
3+
import (
4+
"html/template"
5+
"net/http"
6+
)
7+
8+
func bad(w http.ResponseWriter, r *http.Request) {
9+
r.ParseForm()
10+
username := r.Form.Get("username")
11+
tmpl, _ := template.New("test").Parse(`<b>Hi {{.}}</b>`)
12+
tmpl.Execute(w, template.HTML(username))
13+
}

‎go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXssGood.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package main
2+
3+
import (
4+
"html/template"
5+
"net/http"
6+
)
7+
8+
func good(w http.ResponseWriter, r *http.Request) {
9+
r.ParseForm()
10+
username := r.Form.Get("username")
11+
tmpl, _ := template.New("test").Parse(`<b>Hi {{.}}</b>`)
12+
tmpl.Execute(w, username)
13+
}

‎go/ql/src/Security/CWE-079/StoredXss.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ package main
22

33
import (
44
"io"
5-
"io/ioutil"
65
"net/http"
6+
"os"
77
)
88

99
func ListFiles(w http.ResponseWriter, r *http.Request) {
10-
files, _ := ioutil.ReadDir(".")
10+
files, _ := os.ReadDir(".")
1111

1212
for _, file := range files {
1313
io.WriteString(w, file.Name()+"\n")

‎go/ql/src/Security/CWE-079/StoredXssGood.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package main
33
import (
44
"html"
55
"io"
6-
"io/ioutil"
76
"net/http"
7+
"os"
88
)
99

1010
func ListFiles1(w http.ResponseWriter, r *http.Request) {
11-
files, _ := ioutil.ReadDir(".")
11+
files, _ := os.ReadDir(".")
1212

1313
for _, file := range files {
1414
io.WriteString(w, html.EscapeString(file.Name())+"\n")

‎go/ql/src/change-notes/2025-05-01-html-template-escaping-bypass-xss.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* 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>.

‎go/ql/src/experimental/CWE-79/HTMLTemplateEscapingPassthrough.ql

Lines changed: 0 additions & 153 deletions
This file was deleted.

0 commit comments

Comments
 (0)