Skip to content

Commit a8835f7

Browse files
MashyBaskerunnxt30
MashyBasker
authored and
unnxt30
committed
checker: detect formatted string return (DeepSourceCorp#183)
Signed-off-by: Maharshi Basu <basumaharshi10@gmail.com> Signed-off-by: Unnat Sharma <officialunnat30@gmail.com>
1 parent 7cbedf9 commit a8835f7

File tree

3 files changed

+304
-5
lines changed

3 files changed

+304
-5
lines changed

‎checkers/checker.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ func LoadCustomYamlCheckers(dir string) (map[analysis.Language][]analysis.YamlCh
5858
return checkersMap, err
5959
}
6060

61+
type Analyzer struct {
62+
TestDir string
63+
Analyzers []*goAnalysis.Analyzer
64+
}
65+
6166
func LoadGoCheckers() []*goAnalysis.Analyzer {
6267
analyzers := []*goAnalysis.Analyzer{}
6368

@@ -67,11 +72,6 @@ func LoadGoCheckers() []*goAnalysis.Analyzer {
6772
return analyzers
6873
}
6974

70-
type Analyzer struct {
71-
TestDir string
72-
Analyzers []*goAnalysis.Analyzer
73-
}
74-
7575
func RunAnalyzerTests(analyzerRegistry []Analyzer) (bool, []error) {
7676
passed := true
7777
errors := []error{}

‎checkers/python/flask-format-string-return.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package python
2+
3+
import (
4+
"strings"
5+
6+
sitter "github.com/smacker/go-tree-sitter"
7+
"globstar.dev/analysis"
8+
)
9+
10+
var FlaskFormatStringReturn *analysis.Analyzer = &analysis.Analyzer{
11+
Name: "flask-format-string-return",
12+
Language: analysis.LangPy,
13+
Description: "Returning formatted strings directly from Flask routes creates cross-site scripting vulnerabilities when user input is incorporated without proper escaping. Attackers can inject malicious JavaScript that executes in users' browsers. Flask's template engine with `render_template()` automatically handles proper escaping to prevent these attacks.",
14+
Category: analysis.CategorySecurity,
15+
Severity: analysis.SeverityWarning,
16+
Run: checkFlaskFormatStringReturn,
17+
}
18+
19+
func checkFlaskFormatStringReturn(pass *analysis.Pass) (interface{}, error) {
20+
taintDataMap := make(map[string]bool)
21+
intermVarMap := make(map[string]bool)
22+
23+
// tainted variable from decorated function
24+
analysis.Preorder(pass, func(node *sitter.Node) {
25+
if node.Type() != "decorated_definition" {
26+
return
27+
}
28+
29+
decoratorNode := node.NamedChild(0)
30+
if decoratorNode.Type() != "decorator" {
31+
return
32+
}
33+
34+
callNode := decoratorNode.NamedChild(0)
35+
if callNode.Type() != "call" {
36+
return
37+
}
38+
39+
funcNode := callNode.ChildByFieldName("function")
40+
if funcNode.Type() != "attribute" {
41+
return
42+
}
43+
44+
funcAttr := funcNode.ChildByFieldName("attribute")
45+
if funcAttr.Type() != "identifier" && funcAttr.Content(pass.FileContext.Source) != "route" {
46+
return
47+
}
48+
49+
defNode := node.ChildByFieldName("definition")
50+
if defNode.Type() != "function_definition" {
51+
return
52+
}
53+
54+
paramNode := defNode.ChildByFieldName("parameters")
55+
if paramNode.Type() != "parameters" {
56+
return
57+
}
58+
59+
params := getNamedChildren(paramNode, 0)
60+
61+
for _, p := range params {
62+
taintDataMap[p.Content(pass.FileContext.Source)] = true
63+
}
64+
})
65+
66+
// variable tainted by request
67+
analysis.Preorder(pass, func(node *sitter.Node) {
68+
if node.Type() != "assignment" {
69+
return
70+
}
71+
72+
leftNode := node.ChildByFieldName("left")
73+
rightNode := node.ChildByFieldName("right")
74+
75+
if rightNode == nil {
76+
return
77+
}
78+
79+
if isRequestCall(rightNode, pass.FileContext.Source) {
80+
taintDataMap[leftNode.Content(pass.FileContext.Source)] = true
81+
}
82+
})
83+
84+
// variable of data formatted by tainted variable
85+
analysis.Preorder(pass, func(node *sitter.Node) {
86+
if node.Type() != "assignment" {
87+
return
88+
}
89+
90+
leftNode := node.ChildByFieldName("left")
91+
rightNode := node.ChildByFieldName("right")
92+
93+
if rightNode == nil {
94+
return
95+
}
96+
97+
if isTaintFormatted(rightNode, pass.FileContext.Source, intermVarMap, taintDataMap) {
98+
intermVarMap[leftNode.Content(pass.FileContext.Source)] = true
99+
}
100+
})
101+
102+
// detection
103+
analysis.Preorder(pass, func(node *sitter.Node) {
104+
if node.Type() != "return_statement" {
105+
return
106+
}
107+
returnValNode := node.NamedChild(0)
108+
if isTaintFormatted(returnValNode, pass.FileContext.Source, intermVarMap, taintDataMap) {
109+
pass.Report(pass, node, "Flask route returns formatted string allowing XSS - use render_template() instead")
110+
}
111+
})
112+
113+
return nil, nil
114+
}
115+
116+
func isTaintFormatted(node *sitter.Node, source []byte, intermVarMap, taintDataMap map[string]bool) bool {
117+
switch node.Type() {
118+
case "call":
119+
funcNode := node.ChildByFieldName("function")
120+
if funcNode.Type() != "attribute" {
121+
return false
122+
}
123+
if !strings.HasSuffix(funcNode.Content(source), ".format") {
124+
return false
125+
}
126+
127+
funcArgsListNode := node.ChildByFieldName("arguments")
128+
if funcArgsListNode.Type() != "argument_list" {
129+
return false
130+
}
131+
132+
argsNode := getNamedChildren(funcArgsListNode, 0)
133+
for _, arg := range argsNode {
134+
if isRequestCall(arg, source) || taintDataMap[arg.Content(source)] || intermVarMap[arg.Content(source)] {
135+
return true
136+
}
137+
}
138+
139+
case "binary_operator":
140+
leftNode := node.ChildByFieldName("left")
141+
rightNode := node.ChildByFieldName("right")
142+
143+
if leftNode.Type() != "string" {
144+
return false
145+
}
146+
147+
if rightNode.Type() == "identifier" {
148+
return isRequestCall(rightNode, source) || taintDataMap[rightNode.Content(source)] || intermVarMap[rightNode.Content(source)]
149+
} else if rightNode.Type() == "tuple" {
150+
tupleArgNodes := getNamedChildren(rightNode, 0)
151+
for _, targ := range tupleArgNodes {
152+
if isRequestCall(targ, source) || taintDataMap[targ.Content(source)] || intermVarMap[targ.Content(source)] {
153+
return true
154+
}
155+
}
156+
}
157+
158+
case "string":
159+
stringChildNodes := getNamedChildren(node, 0)
160+
for _, child := range stringChildNodes {
161+
if child.Type() == "interpolation" {
162+
exprNode := child.NamedChild(0)
163+
if isRequestCall(exprNode, source) || taintDataMap[exprNode.Content(source)] || intermVarMap[exprNode.Content(source)] {
164+
return true
165+
}
166+
}
167+
}
168+
169+
case "identifier":
170+
return intermVarMap[node.Content(source)] || taintDataMap[node.Content(source)]
171+
}
172+
173+
return false
174+
}

‎checkers/python/testdata/flask-format-string-return.test.py‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# -*- coding: utf-8 -*-
2+
import os
3+
import sqlite3
4+
5+
from flask import Flask
6+
from flask import redirect
7+
from flask import request
8+
from flask import session
9+
from jinja2 import Template
10+
11+
app = Flask(__name__)
12+
13+
@app.route("/loginpage")
14+
def render_login_page(thing):
15+
# <expect-error>
16+
return '''
17+
<p>{}</p>
18+
<form method="POST" style="margin: 60px auto; width: 140px;">
19+
<p><input name="username" type="text" /></p>
20+
<p><input name="password" type="password" /></p>
21+
<p><input value="Login" type="submit" /></p>
22+
</form>
23+
'''.format(thing)
24+
25+
@app.route("/loginpage2")
26+
def render_login_page2(thing):
27+
# <expect-error>
28+
return '''
29+
<p>%s</p>
30+
<form method="POST" style="margin: 60px auto; width: 140px;">
31+
<p><input name="username" type="text" /></p>
32+
<p><input name="password" type="password" /></p>
33+
<p><input value="Login" type="submit" /></p>
34+
</form>
35+
''' % thing
36+
37+
@app.route("/loginpage3")
38+
def render_login_page3(thing):
39+
# <expect-error>
40+
return '''
41+
<p>%s</p>
42+
<form method="POST" style="margin: 60px auto; width: 140px;">
43+
<p><input name="username" type="text" /></p>
44+
<p><input name="password" type="password" /></p>
45+
<p><input value="Login" type="submit" /></p>
46+
</form>
47+
''' % (thing,)
48+
49+
@app.route("/loginpage4")
50+
def render_login_page4():
51+
thing = "blah"
52+
# the string below is now detected as a literal string after constant
53+
# propagation
54+
# <no-error>
55+
return thing + '''
56+
<form method="POST" style="margin: 60px auto; width: 140px;">
57+
<p><input name="username" type="text" /></p>
58+
<p><input name="password" type="password" /></p>
59+
<p><input value="Login" type="submit" /></p>
60+
</form>
61+
'''
62+
63+
@app.route("/loginpage5")
64+
def render_login_page5():
65+
safe_thing = "blah"
66+
# same, now ok thx to the constant propagation
67+
# <no-error>
68+
return f'''
69+
{safe_thing}
70+
<form method="POST" style="margin: 60px auto; width: 140px;">
71+
<p><input name="username" type="text" /></p>
72+
<p><input name="password" type="password" /></p>
73+
<p><input value="Login" type="submit" /></p>
74+
</form>
75+
'''
76+
77+
@app.route("/loginpage5")
78+
def render_login_page5(thing):
79+
# <expect-error>
80+
return f'''
81+
{thing}
82+
<form method="POST" style="margin: 60px auto; width: 140px;">
83+
<p><input name="username" type="text" /></p>
84+
<p><input name="password" type="password" /></p>
85+
<p><input value="Login" type="submit" /></p>
86+
</form>
87+
'''
88+
89+
# cf. https://raw.githubusercontent.com/Deteriorator/Python-Flask-Web-Development/53be4c48ffbe7d30a1bde5717658f6de81820360/demo/http/app.py
90+
@app.route('/hello')
91+
def hello():
92+
name = request.args.get('name')
93+
if name is None:
94+
name = request.cookies.get('name', 'Human')
95+
respones = '<h1>Hello, %s</h1>' % name
96+
if 'logged_in' in session:
97+
respones += '[Authenticated]'
98+
else:
99+
respones += '[Not Authenticated]'
100+
# <expect-error>
101+
return respones
102+
103+
@app.route('/hello2')
104+
def hello2():
105+
name = request.args.get('name')
106+
if name is None:
107+
name = request.cookies.get('name', 'Human')
108+
respones = '<h1>Hello, {}</h1>'.format(name)
109+
if 'logged_in' in session:
110+
respones += '[Authenticated]'
111+
else:
112+
respones += '[Not Authenticated]'
113+
# <expect-error>
114+
return respones
115+
116+
@app.route('/totally_not_bad')
117+
def totally_not_bad():
118+
# ok
119+
return (
120+
"a" + "\n" +
121+
"b"
122+
)
123+
124+
if __name__ == '__main__':
125+
app.run(debug=True)

0 commit comments

Comments
 (0)