Skip to content

Commit b78c665

Browse files
committed
Python: Model RouteSetup for flask
1 parent d27e695 commit b78c665

File tree

6 files changed

+234
-41
lines changed

6 files changed

+234
-41
lines changed

python/ql/src/experimental/semmle/python/frameworks/Flask.qll

Lines changed: 167 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/**
2-
* Provides classes modeling security-relevant aspects of the `flask` package.
2+
* Provides classes modeling security-relevant aspects of the `flask` PyPI package.
3+
* See https://flask.palletsprojects.com/en/1.1.x/.
34
*/
45

56
private import python
@@ -11,6 +12,10 @@ private import experimental.semmle.python.frameworks.Werkzeug
1112

1213
// for old improved impl see
1314
// https://github.com/github/codeql/blob/9f95212e103c68d0c1dfa4b6f30fb5d53954ccef/python/ql/src/semmle/python/web/flask/Request.qll
15+
/**
16+
* Provides models for the `flask` PyPI package.
17+
* See https://flask.palletsprojects.com/en/1.1.x/.
18+
*/
1419
private module Flask {
1520
/** Gets a reference to the `flask` module. */
1621
DataFlow::Node flask(DataFlow::TypeTracker t) {
@@ -23,6 +28,7 @@ private module Flask {
2328
/** Gets a reference to the `flask` module. */
2429
DataFlow::Node flask() { result = flask(DataFlow::TypeTracker::end()) }
2530

31+
/** Provides models for the `flask` module. */
2632
module flask {
2733
/** Gets a reference to the `flask.request` object. */
2834
DataFlow::Node request(DataFlow::TypeTracker t) {
@@ -32,13 +38,171 @@ private module Flask {
3238
t.startInAttr("request") and
3339
result = flask()
3440
or
35-
exists(DataFlow::TypeTracker t2 | result = flask::request(t2).track(t2, t))
41+
exists(DataFlow::TypeTracker t2 | result = request(t2).track(t2, t))
3642
}
3743

3844
/** Gets a reference to the `flask.request` object. */
39-
DataFlow::Node request() { result = flask::request(DataFlow::TypeTracker::end()) }
45+
DataFlow::Node request() { result = request(DataFlow::TypeTracker::end()) }
46+
47+
/** Gets a reference to the `flask.Flask` class. */
48+
private DataFlow::Node classFlask(DataFlow::TypeTracker t) {
49+
t.start() and
50+
result = DataFlow::importMember("flask", "Flask")
51+
or
52+
t.startInAttr("Flask") and
53+
result = flask()
54+
or
55+
exists(DataFlow::TypeTracker t2 | result = classFlask(t2).track(t2, t))
56+
}
57+
58+
/** Gets a reference to the `flask.Flask` class. */
59+
DataFlow::Node classFlask() { result = classFlask(DataFlow::TypeTracker::end()) }
60+
61+
/** Gets a reference to an instance of `flask.Flask` (a Flask application). */
62+
private DataFlow::Node app(DataFlow::TypeTracker t) {
63+
t.start() and
64+
result.asCfgNode().(CallNode).getFunction() = flask::classFlask().asCfgNode()
65+
or
66+
exists(DataFlow::TypeTracker t2 | result = app(t2).track(t2, t))
67+
}
68+
69+
/** Gets a reference to an instance of `flask.Flask` (a flask application). */
70+
DataFlow::Node app() { result = app(DataFlow::TypeTracker::end()) }
71+
}
72+
73+
// ---------------------------------------------------------------------------
74+
// routing modeling
75+
// ---------------------------------------------------------------------------
76+
/**
77+
* Gets a reference to the attribute `attr_name` of a flask application.
78+
* WARNING: Only holds for a few predefined attributes.
79+
*/
80+
private DataFlow::Node app_attr(DataFlow::TypeTracker t, string attr_name) {
81+
attr_name in ["route", "add_url_rule"] and
82+
t.startInAttr(attr_name) and
83+
result = flask::app()
84+
or
85+
// Due to bad performance when using normal setup with `app_attr(t2, attr_name).track(t2, t)`
86+
// we have inlined that code and forced a join
87+
exists(DataFlow::TypeTracker t2 |
88+
exists(DataFlow::StepSummary summary |
89+
app_attr_first_join(t2, attr_name, result, summary) and
90+
t = t2.append(summary)
91+
)
92+
)
93+
}
94+
95+
pragma[nomagic]
96+
private predicate app_attr_first_join(
97+
DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res, DataFlow::StepSummary summary
98+
) {
99+
DataFlow::StepSummary::step(app_attr(t2, attr_name), res, summary)
100+
}
101+
102+
/**
103+
* Gets a reference to the attribute `attr_name` of a flask application.
104+
* WARNING: Only holds for a few predefined attributes.
105+
*/
106+
private DataFlow::Node app_attr(string attr_name) {
107+
result = app_attr(DataFlow::TypeTracker::end(), attr_name)
108+
}
109+
110+
private string werkzeug_rule_re() {
111+
// since flask uses werkzeug internally, we are using its routing rules from
112+
// https://github.com/pallets/werkzeug/blob/4dc8d6ab840d4b78cbd5789cef91b01e3bde01d5/src/werkzeug/routing.py#L138-L151
113+
result =
114+
"(?<static>[^<]*)<(?:(?<converter>[a-zA-Z_][a-zA-Z0-9_]*)(?:\\((?<args>.*?)\\))?\\:)?(?<variable>[a-zA-Z_][a-zA-Z0-9_]*)>"
115+
}
116+
117+
/** A route setup made by flask (sharing handling of URL patterns). */
118+
abstract private class FlaskRouteSetup extends HTTP::Server::RouteSetup::Range {
119+
override Parameter getARoutedParameter() {
120+
exists(string name |
121+
result = this.getARouteHandler().getArgByName(name) and
122+
exists(string match |
123+
match = this.getUrlPattern().regexpFind(werkzeug_rule_re(), _, _) and
124+
name = match.regexpCapture(werkzeug_rule_re(), 4)
125+
)
126+
)
127+
}
128+
129+
/** Gets the argument used to pass in the URL pattern. */
130+
abstract DataFlow::Node getUrlPatternArg();
131+
132+
override string getUrlPattern() {
133+
exists(StrConst str |
134+
DataFlow::localFlow(DataFlow::exprNode(str), this.getUrlPatternArg()) and
135+
result = str.getText()
136+
)
137+
}
138+
}
139+
140+
/**
141+
* A call to `flask.Flask.route`.
142+
*
143+
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.route
144+
*/
145+
private class FlaskAppRouteCall extends FlaskRouteSetup {
146+
CallNode call;
147+
148+
FlaskAppRouteCall() {
149+
call.getFunction() = app_attr("route").asCfgNode() and
150+
this.asCfgNode() = call
151+
}
152+
153+
override DataFlow::Node getUrlPatternArg() {
154+
exists(ControlFlowNode pattern_arg |
155+
(
156+
pattern_arg = call.getArg(0)
157+
or
158+
pattern_arg = call.getArgByName("rule")
159+
) and
160+
result.asCfgNode() = pattern_arg
161+
)
162+
}
163+
164+
override Function getARouteHandler() { result.getADecorator() = call.getNode() }
165+
}
166+
167+
/**
168+
* A call to `flask.Flask.add_url_rule`.
169+
*
170+
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.add_url_rule
171+
*/
172+
private class FlaskAppAddUrlRule extends FlaskRouteSetup {
173+
CallNode call;
174+
175+
FlaskAppAddUrlRule() {
176+
call.getFunction() = app_attr("add_url_rule").asCfgNode() and
177+
this.asCfgNode() = call
178+
}
179+
180+
override DataFlow::Node getUrlPatternArg() {
181+
exists(ControlFlowNode pattern_arg |
182+
(
183+
pattern_arg = call.getArg(0)
184+
or
185+
pattern_arg = call.getArgByName("rule")
186+
) and
187+
result.asCfgNode() = pattern_arg
188+
)
189+
}
190+
191+
override Function getARouteHandler() {
192+
exists(ControlFlowNode view_func_arg, DataFlow::Node func_src |
193+
view_func_arg = call.getArg(2)
194+
or
195+
view_func_arg = call.getArgByName("view_func")
196+
|
197+
DataFlow::localFlow(func_src, any(DataFlow::Node dest | dest.asCfgNode() = view_func_arg)) and
198+
func_src.asExpr().(CallableExpr) = result.getDefinition()
199+
)
200+
}
40201
}
41202

203+
// ---------------------------------------------------------------------------
204+
// flask.Request taint modeling
205+
// ---------------------------------------------------------------------------
42206
// TODO: Do we even need this class? :|
43207
/**
44208
* A source of remote flow from a flask request.

python/ql/test/experimental/library-tests/frameworks/flask/ConceptsTest.expected

Whitespace-only changes.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import python
2+
import experimental.meta.ConceptsTest

python/ql/test/experimental/library-tests/frameworks/flask/old_test.py

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
from flask import Flask, request, make_response
44
app = Flask(__name__)
55

6-
@app.route("/")
7-
def hello_world():
6+
@app.route("/") # $routeSetup="/"
7+
def hello_world(): # $routeHandler
88
return "Hello World!"
99

1010
from flask.views import MethodView
1111

1212
class MyView(MethodView):
1313

14-
def get(self, user_id):
14+
def get(self, user_id): # $f-:routeHandler
1515
if user_id is None:
1616
# return a list of users
1717
pass
@@ -21,46 +21,46 @@ def get(self, user_id):
2121

2222
the_view = MyView.as_view('my_view')
2323

24-
app.add_url_rule('/the/', defaults={'user_id': None},
24+
app.add_url_rule('/the/', defaults={'user_id': None}, # $routeSetup="/the/"
2525
view_func=the_view, methods=['GET',])
2626

27-
@app.route("/dangerous")
28-
def dangerous():
27+
@app.route("/dangerous") # $routeSetup="/dangerous"
28+
def dangerous(): # $routeHandler
2929
return request.args.get('payload')
3030

31-
@app.route("/dangerous-with-cfg-split")
32-
def dangerous2():
31+
@app.route("/dangerous-with-cfg-split") # $routeSetup="/dangerous-with-cfg-split"
32+
def dangerous2(): # $routeHandler
3333
x = request.form['param0']
3434
if request.method == "POST":
3535
return request.form['param1']
3636
return None
3737

38-
@app.route('/unsafe')
39-
def unsafe():
38+
@app.route("/unsafe") # $routeSetup="/unsafe"
39+
def unsafe(): # $routeHandler
4040
first_name = request.args.get('name', '')
4141
return make_response("Your name is " + first_name)
4242

43-
@app.route('/safe')
44-
def safe():
43+
@app.route("/safe") # $routeSetup="/safe"
44+
def safe(): # $routeHandler
4545
first_name = request.args.get('name', '')
4646
return make_response("Your name is " + escape(first_name))
4747

48-
@app.route('/hello/<name>')
49-
def hello(name):
48+
@app.route("/hello/<name>") # $routeSetup="/hello/<name>"
49+
def hello(name): # $routeHandler $routedParameter=name
5050
return make_response("Your name is " + name)
5151

52-
@app.route('/foo/<path:subpath>')
53-
def foo(subpath):
52+
@app.route("/foo/<path:subpath>") # $routeSetup="/foo/<path:subpath>"
53+
def foo(subpath): # $routeHandler $routedParameter=subpath
5454
return make_response("The subpath is " + subpath)
5555

56-
@app.route('/multiple/') # TODO: not recognized as route
57-
@app.route('/multiple/foo/<foo>') # TODO: not recognized as route
58-
@app.route('/multiple/bar/<bar>')
59-
def multiple(foo=None, bar=None):
56+
@app.route("/multiple/") # $routeSetup="/multiple/"
57+
@app.route("/multiple/foo/<foo>") # $routeSetup="/multiple/foo/<foo>"
58+
@app.route("/multiple/bar/<bar>") # $routeSetup="/multiple/bar/<bar>"
59+
def multiple(foo=None, bar=None): # $routeHandler $routedParameter=foo $routedParameter=bar
6060
return make_response("foo={!r} bar={!r}".format(foo, bar))
6161

62-
@app.route('/complex/<string(length=2):lang_code>')
63-
def complex(lang_code):
62+
@app.route("/complex/<string(length=2):lang_code>") # $routeSetup="/complex/<string(length=2):lang_code>"
63+
def complex(lang_code): # $routeHandler $routedParameter=lang_code
6464
return make_response("lang_code {}".format(lang_code))
6565

6666
if __name__ == "__main__":
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import flask
2+
3+
from flask import Flask, make_response
4+
app = Flask(__name__)
5+
6+
7+
SOME_ROUTE = "/some/route"
8+
@app.route(SOME_ROUTE) # $routeSetup="/some/route"
9+
def some_route(): # $routeHandler
10+
return make_response("some_route")
11+
12+
13+
# TODO: We should be able to handle this one
14+
def index(): # $routeHandler
15+
return make_response("index")
16+
app.add_url_rule('/index', 'index', index) # $routeSetup="/index"
17+
18+
19+
# We don't support this yet, and I think that's OK
20+
def later_set(): # $f-:routeHandler
21+
return make_response("later_set")
22+
app.add_url_rule('/later-set', 'later_set', view_func=None) # $routeSetup="/later-set"
23+
app.view_functions['later_set'] = later_set
24+
25+
26+
if __name__ == "__main__":
27+
app.run(debug=True)

0 commit comments

Comments
 (0)