Skip to content

Commit 62c2316

Browse files
authored
Merge pull request github#14084 from RasmusWL/flask-jsonify
Python: Remove XSS FP from use of `flask.jsonify`
2 parents 6a21fa0 + 49d5100 commit 62c2316

File tree

4 files changed

+28
-3
lines changed

4 files changed

+28
-3
lines changed

python/ql/lib/semmle/python/frameworks/Flask.qll

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,13 @@ module Flask {
179179
* - https://flask.palletsprojects.com/en/2.2.x/api/#flask.json.jsonify
180180
*/
181181
private class FlaskJsonifyCall extends InstanceSource, DataFlow::CallCfgNode {
182-
FlaskJsonifyCall() { this = API::moduleImport("flask").getMember("jsonify").getACall() }
182+
FlaskJsonifyCall() {
183+
this = API::moduleImport("flask").getMember("jsonify").getACall()
184+
or
185+
this = API::moduleImport("flask").getMember("json").getMember("jsonify").getACall()
186+
or
187+
this = FlaskApp::instance().getMember("json").getMember("response").getACall()
188+
}
183189

184190
override DataFlow::Node getBody() { result in [this.getArg(_), this.getArgByName(_)] }
185191

@@ -453,7 +459,8 @@ module Flask {
453459
FlaskRouteHandlerReturn() {
454460
exists(Function routeHandler |
455461
routeHandler = any(FlaskRouteSetup rs).getARequestHandler() and
456-
node = routeHandler.getAReturnValueFlowNode()
462+
node = routeHandler.getAReturnValueFlowNode() and
463+
not this instanceof Flask::Response::InstanceSource
457464
)
458465
}
459466

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Improved _Reflected server-side cross-site scripting_ (`py/reflective-xss`) query to not alert on data passed to `flask.jsonify`. Since these HTTP responses are returned with mime-type `application/json`, they do not pose a security risk for XSS.

python/ql/test/library-tests/frameworks/flask/response_test.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ def html8(): # $requestHandler
6767
@app.route("/jsonify") # $routeSetup="/jsonify"
6868
def jsonify_route(): # $requestHandler
6969
x = "x"; y = "y"; z = "z"
70+
if True:
71+
import flask.json
72+
resp = flask.json.jsonify(x, y, z=z) # $HttpResponse mimetype=application/json responseBody=x responseBody=y responseBody=z
73+
assert resp.mimetype == "application/json"
74+
75+
resp = app.json.response(x, y, z=z) # $HttpResponse mimetype=application/json responseBody=x responseBody=y responseBody=z
76+
assert resp.mimetype == "application/json"
77+
7078
resp = jsonify(x, y, z=z) # $ HttpResponse mimetype=application/json responseBody=x responseBody=y responseBody=z
7179
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
7280

python/ql/test/query-tests/Security/CWE-079-ReflectedXss/reflected_xss.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import json
2-
from flask import Flask, request, make_response, escape
2+
from flask import Flask, request, make_response, escape, jsonify
33

44
app = Flask(__name__)
55

@@ -26,3 +26,9 @@ def unsafe_json():
2626
def safe_json():
2727
data = json.loads(request.data)
2828
return make_response(json.dumps(data), 200, {'Content-Type': 'application/json'}) # OK, FP
29+
30+
31+
@app.route("/jsonify")
32+
def jsonify():
33+
data = request.data
34+
return jsonify(data) # OK, FP

0 commit comments

Comments
 (0)