checker: detect user data formatted string return in flask

checkers/checker.go

@@ -58,6 +58,11 @@ func LoadCustomYamlCheckers(dir string) (map[analysis.Language][]analysis.YamlCh
 	return checkersMap, err
 }
 
+type Analyzer struct {
+	TestDir   string
+	Analyzers []*goAnalysis.Analyzer
+}
+
 func LoadGoCheckers() []*goAnalysis.Analyzer {
 	analyzers := []*goAnalysis.Analyzer{}
 
@@ -67,11 +72,6 @@ func LoadGoCheckers() []*goAnalysis.Analyzer {
 	return analyzers
 }
 
-type Analyzer struct {
-	TestDir   string
-	Analyzers []*goAnalysis.Analyzer
-}
-
 func RunAnalyzerTests(analyzerRegistry []Analyzer) (bool, []error) {
 	passed := true
 	errors := []error{}

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

@@ -0,0 +1,174 @@
+package python
+
+import (
+	"strings"
+
+	sitter "github.com/smacker/go-tree-sitter"
+	"globstar.dev/analysis"
+)
+
+var FlaskFormatStringReturn *analysis.Analyzer = &analysis.Analyzer{
+	Name:        "flask-format-string-return",
+	Language:    analysis.LangPy,
+	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.",
+	Category:    analysis.CategorySecurity,
+	Severity:    analysis.SeverityWarning,
+	Run:         checkFlaskFormatStringReturn,
+}
+
+func checkFlaskFormatStringReturn(pass *analysis.Pass) (interface{}, error) {
+	taintDataMap := make(map[string]bool)
+	intermVarMap := make(map[string]bool)
+
+	// tainted variable from decorated function
+	analysis.Preorder(pass, func(node *sitter.Node) {
+		if node.Type() != "decorated_definition" {
+			return
+		}
+
+		decoratorNode := node.NamedChild(0)
+		if decoratorNode.Type() != "decorator" {
+			return
+		}
+
+		callNode := decoratorNode.NamedChild(0)
+		if callNode.Type() != "call" {
+			return
+		}
+
+		funcNode := callNode.ChildByFieldName("function")
+		if funcNode.Type() != "attribute" {
+			return
+		}
+
+		funcAttr := funcNode.ChildByFieldName("attribute")
+		if funcAttr.Type() != "identifier" && funcAttr.Content(pass.FileContext.Source) != "route" {
+			return
+		}
+
+		defNode := node.ChildByFieldName("definition")
+		if defNode.Type() != "function_definition" {
+			return
+		}
+
+		paramNode := defNode.ChildByFieldName("parameters")
+		if paramNode.Type() != "parameters" {
+			return
+		}
+
+		params := getNamedChildren(paramNode, 0)
+
+		for _, p := range params {
+			taintDataMap[p.Content(pass.FileContext.Source)] = true
+		}
+	})
+
+	// variable tainted by request
+	analysis.Preorder(pass, func(node *sitter.Node) {
+		if node.Type() != "assignment" {
+			return
+		}
+
+		leftNode := node.ChildByFieldName("left")
+		rightNode := node.ChildByFieldName("right")
+
+		if rightNode == nil {
+			return
+		}
+
+		if isRequestCall(rightNode, pass.FileContext.Source) {
+			taintDataMap[leftNode.Content(pass.FileContext.Source)] = true
+		}
+	})
+
+	// variable of data formatted by tainted variable
+	analysis.Preorder(pass, func(node *sitter.Node) {
+		if node.Type() != "assignment" {
+			return
+		}
+
+		leftNode := node.ChildByFieldName("left")
+		rightNode := node.ChildByFieldName("right")
+
+		if rightNode == nil {
+			return
+		}
+
+		if isTaintFormatted(rightNode, pass.FileContext.Source, intermVarMap, taintDataMap) {
+			intermVarMap[leftNode.Content(pass.FileContext.Source)] = true
+		}
+	})
+
+	// detection
+	analysis.Preorder(pass, func(node *sitter.Node) {
+		if node.Type() != "return_statement" {
+			return
+		}
+		returnValNode := node.NamedChild(0)
+		if isTaintFormatted(returnValNode, pass.FileContext.Source, intermVarMap, taintDataMap) {
+			pass.Report(pass, node, "Flask route returns formatted string allowing XSS - use render_template() instead")
+		}
+	})
+
+	return nil, nil
+}
+
+func isTaintFormatted(node *sitter.Node, source []byte, intermVarMap, taintDataMap map[string]bool) bool {
+	switch node.Type() {
+	case "call":
+		funcNode := node.ChildByFieldName("function")
+		if funcNode.Type() != "attribute" {
+			return false
+		}
+		if !strings.HasSuffix(funcNode.Content(source), ".format") {
+			return false
+		}
+
+		funcArgsListNode := node.ChildByFieldName("arguments")
+		if funcArgsListNode.Type() != "argument_list" {
+			return false
+		}
+
+		argsNode := getNamedChildren(funcArgsListNode, 0)
+		for _, arg := range argsNode {
+			if isRequestCall(arg, source) || taintDataMap[arg.Content(source)] || intermVarMap[arg.Content(source)] {
+				return true
+			}
+		}
+
+	case "binary_operator":
+		leftNode := node.ChildByFieldName("left")
+		rightNode := node.ChildByFieldName("right")
+
+		if leftNode.Type() != "string" {
+			return false
+		}
+
+		if rightNode.Type() == "identifier" {
+			return isRequestCall(rightNode, source) || taintDataMap[rightNode.Content(source)] || intermVarMap[rightNode.Content(source)]
+		} else if rightNode.Type() == "tuple" {
+			tupleArgNodes := getNamedChildren(rightNode, 0)
+			for _, targ := range tupleArgNodes {
+				if isRequestCall(targ, source) || taintDataMap[targ.Content(source)] || intermVarMap[targ.Content(source)] {
+					return true
+				}
+			}
+		}
+
+	case "string":
+		stringChildNodes := getNamedChildren(node, 0)
+		for _, child := range stringChildNodes {
+			if child.Type() == "interpolation" {
+				exprNode := child.NamedChild(0)
+				if isRequestCall(exprNode, source) || taintDataMap[exprNode.Content(source)] || intermVarMap[exprNode.Content(source)] {
+					return true
+				}
+			}
+		}
+
+	case "identifier":
+		return intermVarMap[node.Content(source)] || taintDataMap[node.Content(source)]
+	}
+
+	return false
+}

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

@@ -0,0 +1,125 @@
+# -*- coding: utf-8 -*-
+import os
+import sqlite3
+
+from flask import Flask
+from flask import redirect
+from flask import request
+from flask import session
+from jinja2 import Template
+
+app = Flask(__name__)
+
+@app.route("/loginpage")
+def render_login_page(thing):
+    # <expect-error>
+    return '''
+<p>{}</p>
+<form method="POST" style="margin: 60px auto; width: 140px;">
+    <p><input name="username" type="text" /></p>
+    <p><input name="password" type="password" /></p>
+    <p><input value="Login" type="submit" /></p>
+</form>
+    '''.format(thing)
+
+@app.route("/loginpage2")
+def render_login_page2(thing):
+    # <expect-error>
+    return '''
+<p>%s</p>
+<form method="POST" style="margin: 60px auto; width: 140px;">
+    <p><input name="username" type="text" /></p>
+    <p><input name="password" type="password" /></p>
+    <p><input value="Login" type="submit" /></p>
+</form>
+    ''' % thing
+
+@app.route("/loginpage3")
+def render_login_page3(thing):
+    # <expect-error>
+    return '''
+<p>%s</p>
+<form method="POST" style="margin: 60px auto; width: 140px;">
+    <p><input name="username" type="text" /></p>
+    <p><input name="password" type="password" /></p>
+    <p><input value="Login" type="submit" /></p>
+</form>
+    ''' % (thing,)
+
+@app.route("/loginpage4")
+def render_login_page4():
+    thing = "blah"
+    # the string below is now detected as a literal string after constant
+    # propagation
+    # <no-error>
+    return thing + '''
+<form method="POST" style="margin: 60px auto; width: 140px;">
+    <p><input name="username" type="text" /></p>
+    <p><input name="password" type="password" /></p>
+    <p><input value="Login" type="submit" /></p>
+</form>
+    '''
+
+@app.route("/loginpage5")
+def render_login_page5():
+    safe_thing = "blah"
+    # same, now ok thx to the constant propagation
+    # <no-error>
+    return f'''
+{safe_thing}
+<form method="POST" style="margin: 60px auto; width: 140px;">
+    <p><input name="username" type="text" /></p>
+    <p><input name="password" type="password" /></p>
+    <p><input value="Login" type="submit" /></p>
+</form>
+    '''
+
+@app.route("/loginpage5")
+def render_login_page5(thing):
+    # <expect-error>
+    return f'''
+{thing}
+<form method="POST" style="margin: 60px auto; width: 140px;">
+    <p><input name="username" type="text" /></p>
+    <p><input name="password" type="password" /></p>
+    <p><input value="Login" type="submit" /></p>
+</form>
+    '''
+
+# cf. https://raw.githubusercontent.com/Deteriorator/Python-Flask-Web-Development/53be4c48ffbe7d30a1bde5717658f6de81820360/demo/http/app.py
+@app.route('/hello')
+def hello():
+    name = request.args.get('name')
+    if name is None:
+        name = request.cookies.get('name', 'Human')
+    respones = '<h1>Hello, %s</h1>' % name
+    if 'logged_in' in session:
+        respones += '[Authenticated]'
+    else:
+        respones += '[Not Authenticated]'
+    # <expect-error>
+    return respones
+
+@app.route('/hello2')
+def hello2():
+    name = request.args.get('name')
+    if name is None:
+        name = request.cookies.get('name', 'Human')
+    respones = '<h1>Hello, {}</h1>'.format(name)
+    if 'logged_in' in session:
+        respones += '[Authenticated]'
+    else:
+        respones += '[Not Authenticated]'
+    # <expect-error>
+    return respones
+
+@app.route('/totally_not_bad')
+def totally_not_bad():
+    # ok
+    return (
+      "a" + "\n" +
+      "b"
+    )
+
+if __name__ == '__main__':
+    app.run(debug=True)