mirror of
https://github.com/github/codeql.git
synced 2026-05-01 03:35:13 +02:00
Python: Add some tricky tests of return in flask route handler
In these cases the `return` might end up creating a new HTTP response, so they need to be modeled as such. Initially I created a very naive solution that didn't handle either tricky_return1 or tricky_return2. The interaction in tricky_return2/helper highlighted for me that to handle this properly, due to the fact that the flow is across functions, we either need to use a global dataflow/taint-tracking configuration, or some clever use of type-trackers. In the end, this extra effort for not modeling all returns in a flask route handler as a creation of a HTTP response doesn't really seem to be worth it (at least not right now). Sicne we use it with taint-tracking for the Reflected XSS query, and use a HTTP response _creation_ as the sink (without propagating taint to the HTTP response), we won't get into trouble where we report a path to BOTH `make_response(...)` and the `return` ``` resp = make_response(...) return resp ``` If we change this setup in the future, we will probably need to do something to avoid this double-path reporting.
This commit is contained in:
Rasmus Wriedt Larsen
@@ -1,48 +0,0 @@
|
||||
| response_test.py:19:12:19:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:19:12:19:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:19:12:19:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:25:12:25:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:25:12:25:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:25:12:25:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:35:12:35:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:35:12:35:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:35:12:35:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:43:12:43:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:43:12:43:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:43:12:43:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:51:12:51:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:51:12:51:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:51:12:51:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:58:12:58:19 | ControlFlowNode for response | Unexpected result: HttpResponse= |
|
||||
| response_test.py:58:12:58:19 | ControlFlowNode for response | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:58:12:58:19 | ControlFlowNode for response | Unexpected result: responseBody=response |
|
||||
| response_test.py:70:12:70:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:70:12:70:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:70:12:70:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:77:12:77:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:77:12:77:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:77:12:77:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:87:12:87:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:87:12:87:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:87:12:87:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:93:12:93:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:93:12:93:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:93:12:93:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:100:12:100:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:100:12:100:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:100:12:100:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:107:12:107:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:107:12:107:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:107:12:107:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:115:12:115:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:115:12:115:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:115:12:115:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:123:12:123:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:123:12:123:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:123:12:123:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:131:12:131:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:131:12:131:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:131:12:131:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
| response_test.py:139:12:139:15 | ControlFlowNode for resp | Unexpected result: HttpResponse= |
|
||||
| response_test.py:139:12:139:15 | ControlFlowNode for resp | Unexpected result: mimetype=text/html |
|
||||
| response_test.py:139:12:139:15 | ControlFlowNode for resp | Unexpected result: responseBody=resp |
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import json
|
||||
|
||||
from flask import Flask, make_response, jsonify, Response
|
||||
from flask import Flask, make_response, jsonify, Response, request
|
||||
|
||||
app = Flask(__name__)
|
||||
|
||||
@@ -16,13 +16,13 @@ def html2(): # $routeHandler
|
||||
# show that we recognize the response creation, and not the return (hopefully). (and
|
||||
# do the same in the following of the file)
|
||||
resp = make_response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/html3") # $routeSetup="/html3"
|
||||
def html3(): # $routeHandler
|
||||
resp = app.make_response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
# TODO: Create test-cases for the many ways that `make_response` can be used
|
||||
@@ -32,7 +32,7 @@ def html3(): # $routeHandler
|
||||
@app.route("/html4") # $routeSetup="/html4"
|
||||
def html4(): # $routeHandler
|
||||
resp = Response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/html5") # $routeSetup="/html5"
|
||||
@@ -40,7 +40,7 @@ def html5(): # $routeHandler
|
||||
# note: flask.Flask.response_class is set to `flask.Response` by default.
|
||||
# it can be overridden, but we don't try to handle that right now.
|
||||
resp = Flask.response_class("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/html6") # $routeSetup="/html6"
|
||||
@@ -48,14 +48,37 @@ def html6(): # $routeHandler
|
||||
# note: app.response_class (flask.Flask.response_class) is set to `flask.Response` by default.
|
||||
# it can be overridden, but we don't try to handle that right now.
|
||||
resp = app.response_class("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/jsonify") # $routeSetup="/jsonify"
|
||||
def jsonify_route(): # $routeHandler
|
||||
data = {"foo": "bar"}
|
||||
response = jsonify(data) # $f-:HttpResponse $f-:mimetype=application/json $f-:responseBody=data
|
||||
return response
|
||||
resp = jsonify(data) # $f-:HttpResponse $f-:mimetype=application/json $f-:responseBody=data
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
################################################################################
|
||||
# Tricky return handling
|
||||
################################################################################
|
||||
|
||||
@app.route("/tricky-return1") # $routeSetup="/tricky-return1"
|
||||
def tricky_return1(): # $routeHandler
|
||||
if "raw" in request.args:
|
||||
resp = "<h1>hellu</h1>"
|
||||
else:
|
||||
resp = make_response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
return resp # $HttpResponse $mimetype=text/html $responseBody=resp
|
||||
|
||||
def helper():
|
||||
if "raw" in request.args:
|
||||
return "<h1>hellu</h1>"
|
||||
else:
|
||||
return make_response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
|
||||
@app.route("/tricky-return2") # $routeSetup="/tricky-return2"
|
||||
def tricky_return2(): # $routeHandler
|
||||
resp = helper()
|
||||
return resp # $HttpResponse $mimetype=text/html $responseBody=resp
|
||||
|
||||
|
||||
################################################################################
|
||||
@@ -67,14 +90,14 @@ def jsonify_route(): # $routeHandler
|
||||
def response_modification1(): # $routeHandler
|
||||
resp = make_response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
resp.content_type = "text/plain" # $f-:HttpResponse $f-:mimetype=text/plain
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/response-modification2") # $routeSetup="/content-type/response-modification2"
|
||||
def response_modification2(): # $routeHandler
|
||||
resp = make_response("<h1>hello</h1>") # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
|
||||
resp.headers["content-type"] = "text/plain" # $f-:HttpResponse $f-:mimetype=text/plain
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
# Exploration of mimetype/content_type/headers arguments to `app.response_class` -- things can get tricky
|
||||
@@ -84,27 +107,27 @@ def response_modification2(): # $routeHandler
|
||||
@app.route("/content-type/Response1") # $routeSetup="/content-type/Response1"
|
||||
def Response1(): # $routeHandler
|
||||
resp = Response("<h1>hello</h1>", mimetype="text/plain") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/Response2") # $routeSetup="/content-type/Response2"
|
||||
def Response2(): # $routeHandler
|
||||
resp = Response("<h1>hello</h1>", content_type="text/plain; charset=utf-8") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/Response3") # $routeSetup="/content-type/Response3"
|
||||
def Response3(): # $routeHandler
|
||||
# content_type argument takes priority (and result is text/plain)
|
||||
resp = Response("<h1>hello</h1>", content_type="text/plain; charset=utf-8", mimetype="text/html") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/Response4") # $routeSetup="/content-type/Response4"
|
||||
def Response4(): # $routeHandler
|
||||
# note: capitalization of Content-Type does not matter
|
||||
resp = Response("<h1>hello</h1>", headers={"Content-TYPE": "text/plain"}) # $HttpResponse $f+:mimetype=text/html $f-:mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/Response5") # $routeSetup="/content-type/Response5"
|
||||
@@ -112,7 +135,7 @@ def Response5(): # $routeHandler
|
||||
# content_type argument takes priority (and result is text/plain)
|
||||
# note: capitalization of Content-Type does not matter
|
||||
resp = Response("<h1>hello</h1>", headers={"Content-TYPE": "text/html"}, content_type="text/plain; charset=utf-8") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/Response6") # $routeSetup="/content-type/Response6"
|
||||
@@ -120,7 +143,7 @@ def Response6(): # $routeHandler
|
||||
# mimetype argument takes priority over header (and result is text/plain)
|
||||
# note: capitalization of Content-Type does not matter
|
||||
resp = Response("<h1>hello</h1>", headers={"Content-TYPE": "text/html"}, mimetype="text/plain") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/Flask-response-class") # $routeSetup="/content-type/Flask-response-class"
|
||||
@@ -128,7 +151,7 @@ def Flask_response_class(): # $routeHandler
|
||||
# note: flask.Flask.response_class is set to `flask.Response` by default.
|
||||
# it can be overridden, but we don't try to handle that right now.
|
||||
resp = Flask.response_class("<h1>hello</h1>", mimetype="text/plain") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
@app.route("/content-type/app-response-class") # $routeSetup="/content-type/app-response-class"
|
||||
@@ -136,7 +159,7 @@ def app_response_class(): # $routeHandler
|
||||
# note: app.response_class (flask.Flask.response_class) is set to `flask.Response` by default.
|
||||
# it can be overridden, but we don't try to handle that right now.
|
||||
resp = app.response_class("<h1>hello</h1>", mimetype="text/plain") # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
|
||||
return resp
|
||||
return resp # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
|
||||
|
||||
|
||||
# TODO: add tests for setting status code
|
||||
|
||||
Reference in New Issue
Block a user